[CVAT-UI] Critical fixes (#1874)

* Fixed redux types * Redesigned approach to close job. Previous variant didn't work properly with GlobalErrorBoundary * Fixed: cannot read property shapeType of undefined * Cannot read property 'pinned' of undefined * Do not iterate invisible objects (zLayer) in aam * Keep cursor on the same position when editing text * Do not select hidden shapes when grouping * Updated version * Fixed host
cvat-ai · Jul 9, 2020 · 2a5cfcc · 2a5cfcc
1 parent bcdcaaf
commit 2a5cfcc
Show file tree

Hide file tree

Showing 16 changed files with 54 additions and 47 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,6 +19,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 - Some objects aren't shown on canvas sometimes. For example after propagation on of objects is invisible (<https://github.com/opencv/cvat/pull/1834>)
+- CVAT doesn't offer to restore state after an error (<https://github.com/opencv/cvat/pull/1874>)
+- Cannot read property 'shapeType' of undefined because of zOrder related issues (<https://github.com/opencv/cvat/pull/1874>)
+- Cannot read property 'pinned' of undefined because of zOrder related issues (<https://github.com/opencv/cvat/pull/1874>)
+- Do not iterate over hidden objects in aam (which are invisible because of zOrder) (<https://github.com/opencv/cvat/pull/1874>)
+- Cursor position is reset after changing a text field (<https://github.com/opencv/cvat/pull/1874>)
+- Hidden points and cuboids can be selected to be groupped (<https://github.com/opencv/cvat/pull/1874>)
 - `outside` annotations should not be in exported images (<https://github.com/opencv/cvat/issues/1620>)
 
 ### Security

diff --git a/cvat-canvas/package-lock.json b/cvat-canvas/package-lock.json
diff --git a/cvat-canvas/package.json b/cvat-canvas/package.json
@@ -1,6 +1,6 @@
 {
   "name": "cvat-canvas",
-  "version": "1.2.2",
+  "version": "2.0.0",
   "description": "Part of Computer Vision Annotation Tool which presents its canvas library",
   "main": "src/canvas.ts",
   "scripts": {

diff --git a/cvat-canvas/src/typescript/canvas.ts b/cvat-canvas/src/typescript/canvas.ts
@@ -36,8 +36,7 @@ const CanvasVersion = pjson.version;
 
 interface Canvas {
     html(): HTMLDivElement;
-    setZLayer(zLayer: number | null): void;
-    setup(frameData: any, objectStates: any[]): void;
+    setup(frameData: any, objectStates: any[], zLayer?: number): void;
     activate(clientID: number | null, attributeID?: number): void;
     rotate(rotationAngle: number): void;
     focus(clientID: number, padding?: number): void;
@@ -76,12 +75,8 @@ class CanvasImpl implements Canvas {
         return this.view.html();
     }
 
-    public setZLayer(zLayer: number | null): void {
-        this.model.setZLayer(zLayer);
-    }
-
-    public setup(frameData: any, objectStates: any[]): void {
-        this.model.setup(frameData, objectStates);
+    public setup(frameData: any, objectStates: any[], zLayer = 0): void {
+        this.model.setup(frameData, objectStates, zLayer);
     }
 
     public fitCanvas(): void {

diff --git a/cvat-canvas/src/typescript/canvasModel.ts b/cvat-canvas/src/typescript/canvasModel.ts
@@ -98,7 +98,6 @@ export enum UpdateReasons {
     IMAGE_FITTED = 'image_fitted',
     IMAGE_MOVED = 'image_moved',
     GRID_UPDATED = 'grid_updated',
-    SET_Z_LAYER = 'set_z_layer',
 
     OBJECTS_UPDATED = 'objects_updated',
     SHAPE_ACTIVATED = 'shape_activated',
@@ -148,11 +147,10 @@ export interface CanvasModel {
     geometry: Geometry;
     mode: Mode;
 
-    setZLayer(zLayer: number | null): void;
     zoom(x: number, y: number, direction: number): void;
     move(topOffset: number, leftOffset: number): void;
 
-    setup(frameData: any, objectStates: any[]): void;
+    setup(frameData: any, objectStates: any[], zLayer: number): void;
     activate(clientID: number | null, attributeID: number | null): void;
     rotate(rotationAngle: number): void;
     focus(clientID: number, padding: number): void;
@@ -258,11 +256,6 @@ export class CanvasModelImpl extends MasterImpl implements CanvasModel {
         };
     }
 
-    public setZLayer(zLayer: number | null): void {
-        this.data.zLayer = zLayer;
-        this.notify(UpdateReasons.SET_Z_LAYER);
-    }
-
     public zoom(x: number, y: number, direction: number): void {
         const oldScale: number = this.data.scale;
         const newScale: number = direction > 0 ? oldScale * 6 / 5 : oldScale * 5 / 6;
@@ -337,14 +330,15 @@ export class CanvasModelImpl extends MasterImpl implements CanvasModel {
         this.notify(UpdateReasons.ZOOM_CANVAS);
     }
 
-    public setup(frameData: any, objectStates: any[]): void {
+    public setup(frameData: any, objectStates: any[], zLayer: number): void {
         if (this.data.imageID !== frameData.number) {
             if ([Mode.EDIT, Mode.DRAG, Mode.RESIZE].includes(this.data.mode)) {
                 throw Error(`Canvas is busy. Action: ${this.data.mode}`);
             }
         }
 
         if (frameData.number === this.data.imageID) {
+            this.data.zLayer = zLayer;
             this.data.objects = objectStates;
             this.notify(UpdateReasons.OBJECTS_UPDATED);
             return;
@@ -369,6 +363,7 @@ export class CanvasModelImpl extends MasterImpl implements CanvasModel {
 
             this.data.image = data;
             this.notify(UpdateReasons.IMAGE_CHANGED);
+            this.data.zLayer = zLayer;
             this.data.objects = objectStates;
             this.notify(UpdateReasons.OBJECTS_UPDATED);
         }).catch((exception: any): void => {
@@ -388,9 +383,9 @@ export class CanvasModelImpl extends MasterImpl implements CanvasModel {
         }
 
         if (typeof (clientID) === 'number') {
-            const [state] = this.data.objects
+            const [state] = this.objects
                 .filter((_state: any): boolean => _state.clientID === clientID);
-            if (!['rectangle', 'polygon', 'polyline', 'points', 'cuboid'].includes(state.shapeType)) {
+            if (!state || state.objectType === 'tag') {
                 return;
             }
         }

diff --git a/cvat-canvas/src/typescript/canvasView.ts b/cvat-canvas/src/typescript/canvasView.ts
@@ -991,7 +991,7 @@ export class CanvasViewImpl implements CanvasView, Listener {
             }
         } else if (reason === UpdateReasons.IMAGE_MOVED) {
             this.moveCanvas();
-        } else if ([UpdateReasons.OBJECTS_UPDATED, UpdateReasons.SET_Z_LAYER].includes(reason)) {
+        } else if ([UpdateReasons.OBJECTS_UPDATED].includes(reason)) {
             if (this.mode === Mode.GROUP) {
                 this.groupHandler.resetSelectedObjects();
             }
@@ -1128,7 +1128,6 @@ export class CanvasViewImpl implements CanvasView, Listener {
         if (model.imageBitmap
             && [UpdateReasons.IMAGE_CHANGED,
                 UpdateReasons.OBJECTS_UPDATED,
-                UpdateReasons.SET_Z_LAYER,
             ].includes(reason)
         ) {
             this.redrawBitmap();

diff --git a/cvat-canvas/src/typescript/groupHandler.ts b/cvat-canvas/src/typescript/groupHandler.ts
@@ -95,7 +95,9 @@ export class GroupHandlerImpl implements GroupHandler {
             this.selectionRect = null;
 
             const box = this.getSelectionBox(event);
-            const shapes = (this.canvas.select('.cvat_canvas_shape') as any).members;
+            const shapes = (this.canvas.select('.cvat_canvas_shape') as any).members.filter(
+                (shape: SVG.Shape): boolean => !shape.hasClass('cvat_canvas_hidden'),
+            );
             for (const shape of shapes) {
                 // TODO: Doesn't work properly for groups
                 const bbox = shape.bbox();

diff --git a/cvat-ui/package-lock.json b/cvat-ui/package-lock.json
diff --git a/cvat-ui/package.json b/cvat-ui/package.json
@@ -1,6 +1,6 @@
 {
   "name": "cvat-ui",
-  "version": "1.6.0",
+  "version": "1.6.1",
   "description": "CVAT single-page application",
   "main": "src/index.tsx",
   "scripts": {

diff --git a/cvat-ui/src/components/annotation-page/annotation-page.tsx b/cvat-ui/src/components/annotation-page/annotation-page.tsx
@@ -4,6 +4,7 @@
 
 import './styles.scss';
 import React, { useEffect } from 'react';
+import { useHistory } from 'react-router';
 import Layout from 'antd/lib/layout';
 import Spin from 'antd/lib/spin';
 import Result from 'antd/lib/result';
@@ -20,6 +21,7 @@ interface Props {
     fetching: boolean;
     getJob(): void;
     saveLogs(): void;
+    closeJob(): void;
     workspace: Workspace;
 }
 
@@ -28,10 +30,12 @@ export default function AnnotationPageComponent(props: Props): JSX.Element {
         job,
         fetching,
         getJob,
+        closeJob,
         saveLogs,
         workspace,
     } = props;
 
+    const history = useHistory();
     useEffect(() => {
         saveLogs();
         const root = window.document.getElementById('root');
@@ -44,6 +48,10 @@ export default function AnnotationPageComponent(props: Props): JSX.Element {
             if (root) {
                 root.style.minHeight = '';
             }
+
+            if (!history.location.pathname.includes('/jobs')) {
+                closeJob();
+            }
         };
     }, []);
 

diff --git a/...ribute-annotation-workspace/attribute-annotation-sidebar/attribute-annotation-sidebar.tsx b/...ribute-annotation-workspace/attribute-annotation-sidebar/attribute-annotation-sidebar.tsx
@@ -40,6 +40,7 @@ interface StateToProps {
     normalizedKeyMap: Record<string, string>;
     canvasInstance: Canvas;
     canvasIsReady: boolean;
+    curZLayer: number;
 }
 
 interface DispatchToProps {
@@ -59,6 +60,9 @@ function mapStateToProps(state: CombinedState): StateToProps {
                 activatedStateID,
                 activatedAttributeID,
                 states,
+                zLayer: {
+                    cur,
+                },
             },
             job: {
                 instance: jobInstance,
@@ -85,6 +89,7 @@ function mapStateToProps(state: CombinedState): StateToProps {
         normalizedKeyMap,
         canvasInstance,
         canvasIsReady,
+        curZLayer: cur,
     };
 }
 
@@ -116,9 +121,12 @@ function AttributeAnnotationSidebar(props: StateToProps & DispatchToProps): JSX.
         normalizedKeyMap,
         canvasInstance,
         canvasIsReady,
+        curZLayer,
     } = props;
 
-    const filteredStates = states.filter((state) => !state.outside && !state.hidden);
+    const filteredStates = states.filter((state) => !state.outside
+        && !state.hidden
+        && state.zOrder <= curZLayer);
     const [labelAttrMap, setLabelAttrMap] = useState(
         labels.reduce((acc, label): LabelAttrMap => {
             acc[label.id] = label.attributes.length ? label.attributes[0] : null;

diff --git a/cvat-ui/src/components/annotation-page/standard-workspace/canvas-wrapper.tsx b/cvat-ui/src/components/annotation-page/standard-workspace/canvas-wrapper.tsx
@@ -102,7 +102,6 @@ export default class CanvasWrapperComponent extends React.PureComponent<Props> {
             automaticBordering,
             showObjectsTextAlways,
             canvasInstance,
-            curZLayer,
         } = this.props;
 
         // It's awful approach from the point of view React
@@ -116,7 +115,6 @@ export default class CanvasWrapperComponent extends React.PureComponent<Props> {
             undefinedAttrValue: consts.UNDEFINED_ATTRIBUTE_VALUE,
             displayAllText: showObjectsTextAlways,
         });
-        canvasInstance.setZLayer(curZLayer);
 
         this.initialSetup();
         this.updateCanvas();
@@ -217,17 +215,15 @@ export default class CanvasWrapperComponent extends React.PureComponent<Props> {
             }
         }
 
-        if (prevProps.curZLayer !== curZLayer) {
-            canvasInstance.setZLayer(curZLayer);
-        }
-
-        if (prevProps.annotations !== annotations || prevProps.frameData !== frameData) {
+        if (prevProps.annotations !== annotations
+            || prevProps.frameData !== frameData
+            || prevProps.curZLayer !== curZLayer) {
             this.updateCanvas();
         }
 
         if (prevProps.frame !== frameData.number
-            && ((resetZoom && workspace !== Workspace.ATTRIBUTE_ANNOTATION) ||
-            workspace === Workspace.TAG_ANNOTATION)
+            && ((resetZoom && workspace !== Workspace.ATTRIBUTE_ANNOTATION)
+            || workspace === Workspace.TAG_ANNOTATION)
         ) {
             canvasInstance.html().addEventListener('canvas.setup', () => {
                 canvasInstance.fit();
@@ -636,14 +632,15 @@ export default class CanvasWrapperComponent extends React.PureComponent<Props> {
 
     private updateCanvas(): void {
         const {
+            curZLayer,
             annotations,
             frameData,
             canvasInstance,
         } = this.props;
 
         if (frameData !== null) {
             canvasInstance.setup(frameData, annotations
-                .filter((e) => e.objectType !== ObjectType.TAG));
+                .filter((e) => e.objectType !== ObjectType.TAG), curZLayer);
         }
     }
 

diff --git a/.../components/annotation-page/standard-workspace/objects-side-bar/object-item-attribute.tsx b/.../components/annotation-page/standard-workspace/objects-side-bar/object-item-attribute.tsx
@@ -164,7 +164,7 @@ function ItemAttributeComponent(props: Props): JSX.Element {
                     onChange={(event: React.ChangeEvent<HTMLInputElement>): void => {
                         changeAttribute(attrID, event.target.value);
                     }}
-                    value={attrValue}
+                    defaultValue={attrValue}
                     className='cvat-object-item-text-attribute'
                 />
             </Col>

diff --git a/cvat-ui/src/containers/annotation-page/annotation-page.tsx b/cvat-ui/src/containers/annotation-page/annotation-page.tsx
@@ -7,7 +7,7 @@ import { withRouter } from 'react-router-dom';
 import { RouteComponentProps } from 'react-router';
 
 import AnnotationPageComponent from 'components/annotation-page/annotation-page';
-import { getJobAsync, saveLogsAsync } from 'actions/annotation-actions';
+import { getJobAsync, saveLogsAsync, closeJob as closeJobAction } from 'actions/annotation-actions';
 
 import { CombinedState, Workspace } from 'reducers/interfaces';
 
@@ -25,6 +25,7 @@ interface StateToProps {
 interface DispatchToProps {
     getJob(): void;
     saveLogs(): void;
+    closeJob(): void;
 }
 
 function mapStateToProps(state: CombinedState, own: OwnProps): StateToProps {
@@ -83,6 +84,9 @@ function mapDispatchToProps(dispatch: any, own: OwnProps): DispatchToProps {
         saveLogs(): void {
             dispatch(saveLogsAsync());
         },
+        closeJob(): void {
+            dispatch(closeJobAction());
+        },
     };
 }
 

diff --git a/cvat-ui/src/containers/annotation-page/top-bar/top-bar.tsx b/cvat-ui/src/containers/annotation-page/top-bar/top-bar.tsx
@@ -22,7 +22,6 @@ import {
     searchAnnotationsAsync,
     changeWorkspace as changeWorkspaceAction,
     activateObject,
-    closeJob as closeJobAction,
 } from 'actions/annotation-actions';
 import { Canvas } from 'cvat-canvas-wrapper';
 
@@ -59,7 +58,6 @@ interface DispatchToProps {
     redo(sessionInstance: any, frameNumber: any): void;
     searchAnnotations(sessionInstance: any, frameFrom: any, frameTo: any): void;
     changeWorkspace(workspace: Workspace): void;
-    closeJob(): void;
 }
 
 function mapStateToProps(state: CombinedState): StateToProps {
@@ -155,9 +153,6 @@ function mapDispatchToProps(dispatch: any): DispatchToProps {
             dispatch(activateObject(null, null));
             dispatch(changeWorkspaceAction(workspace));
         },
-        closeJob(): void {
-            dispatch(closeJobAction());
-        },
     };
 }
 
@@ -246,11 +241,9 @@ class AnnotationTopBarContainer extends React.PureComponent<Props> {
     }
 
     public componentWillUnmount(): void {
-        const { closeJob } = this.props;
         window.clearInterval(this.autoSaveInterval);
         window.removeEventListener('beforeunload', this.beforeUnloadCallback);
         this.unblock();
-        closeJob();
     }
 
     private undo = (): void => {

diff --git a/cvat-ui/src/utils/redux.ts b/cvat-ui/src/utils/redux.ts
@@ -20,7 +20,7 @@ export function createAction<T extends string, P>(
 
 export type ActionUnion<A extends ActionCreatorsMapObject> = ReturnType<A[keyof A]>;
 
-export type ThunkAction<R = void, A extends Action = AnyAction>
+export type ThunkAction<R = {}, A extends Action = AnyAction>
     = _ThunkAction<R, CombinedState, {}, A>;
 
 export type ThunkDispatch<E = {}, A extends Action = AnyAction>