[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
main
Boris Sekachev 6 years ago committed by GitHub
parent bcdcaaf459
commit 2a5cfcc657
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -19,6 +19,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed ### 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>) - 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>) - `outside` annotations should not be in exported images (<https://github.com/opencv/cvat/issues/1620>)
### Security ### Security

@ -1,6 +1,6 @@
{ {
"name": "cvat-canvas", "name": "cvat-canvas",
"version": "1.2.2", "version": "2.0.0",
"lockfileVersion": 1, "lockfileVersion": 1,
"requires": true, "requires": true,
"dependencies": { "dependencies": {

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

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

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

@ -991,7 +991,7 @@ export class CanvasViewImpl implements CanvasView, Listener {
} }
} else if (reason === UpdateReasons.IMAGE_MOVED) { } else if (reason === UpdateReasons.IMAGE_MOVED) {
this.moveCanvas(); 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) { if (this.mode === Mode.GROUP) {
this.groupHandler.resetSelectedObjects(); this.groupHandler.resetSelectedObjects();
} }
@ -1128,7 +1128,6 @@ export class CanvasViewImpl implements CanvasView, Listener {
if (model.imageBitmap if (model.imageBitmap
&& [UpdateReasons.IMAGE_CHANGED, && [UpdateReasons.IMAGE_CHANGED,
UpdateReasons.OBJECTS_UPDATED, UpdateReasons.OBJECTS_UPDATED,
UpdateReasons.SET_Z_LAYER,
].includes(reason) ].includes(reason)
) { ) {
this.redrawBitmap(); this.redrawBitmap();

@ -95,7 +95,9 @@ export class GroupHandlerImpl implements GroupHandler {
this.selectionRect = null; this.selectionRect = null;
const box = this.getSelectionBox(event); 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) { for (const shape of shapes) {
// TODO: Doesn't work properly for groups // TODO: Doesn't work properly for groups
const bbox = shape.bbox(); const bbox = shape.bbox();

@ -1,6 +1,6 @@
{ {
"name": "cvat-ui", "name": "cvat-ui",
"version": "1.6.0", "version": "1.6.1",
"lockfileVersion": 1, "lockfileVersion": 1,
"requires": true, "requires": true,
"dependencies": { "dependencies": {

@ -1,6 +1,6 @@
{ {
"name": "cvat-ui", "name": "cvat-ui",
"version": "1.6.0", "version": "1.6.1",
"description": "CVAT single-page application", "description": "CVAT single-page application",
"main": "src/index.tsx", "main": "src/index.tsx",
"scripts": { "scripts": {

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

@ -40,6 +40,7 @@ interface StateToProps {
normalizedKeyMap: Record<string, string>; normalizedKeyMap: Record<string, string>;
canvasInstance: Canvas; canvasInstance: Canvas;
canvasIsReady: boolean; canvasIsReady: boolean;
curZLayer: number;
} }
interface DispatchToProps { interface DispatchToProps {
@ -59,6 +60,9 @@ function mapStateToProps(state: CombinedState): StateToProps {
activatedStateID, activatedStateID,
activatedAttributeID, activatedAttributeID,
states, states,
zLayer: {
cur,
},
}, },
job: { job: {
instance: jobInstance, instance: jobInstance,
@ -85,6 +89,7 @@ function mapStateToProps(state: CombinedState): StateToProps {
normalizedKeyMap, normalizedKeyMap,
canvasInstance, canvasInstance,
canvasIsReady, canvasIsReady,
curZLayer: cur,
}; };
} }
@ -116,9 +121,12 @@ function AttributeAnnotationSidebar(props: StateToProps & DispatchToProps): JSX.
normalizedKeyMap, normalizedKeyMap,
canvasInstance, canvasInstance,
canvasIsReady, canvasIsReady,
curZLayer,
} = props; } = 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( const [labelAttrMap, setLabelAttrMap] = useState(
labels.reduce((acc, label): LabelAttrMap => { labels.reduce((acc, label): LabelAttrMap => {
acc[label.id] = label.attributes.length ? label.attributes[0] : null; acc[label.id] = label.attributes.length ? label.attributes[0] : null;

@ -102,7 +102,6 @@ export default class CanvasWrapperComponent extends React.PureComponent<Props> {
automaticBordering, automaticBordering,
showObjectsTextAlways, showObjectsTextAlways,
canvasInstance, canvasInstance,
curZLayer,
} = this.props; } = this.props;
// It's awful approach from the point of view React // 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, undefinedAttrValue: consts.UNDEFINED_ATTRIBUTE_VALUE,
displayAllText: showObjectsTextAlways, displayAllText: showObjectsTextAlways,
}); });
canvasInstance.setZLayer(curZLayer);
this.initialSetup(); this.initialSetup();
this.updateCanvas(); this.updateCanvas();
@ -217,17 +215,15 @@ export default class CanvasWrapperComponent extends React.PureComponent<Props> {
} }
} }
if (prevProps.curZLayer !== curZLayer) { if (prevProps.annotations !== annotations
canvasInstance.setZLayer(curZLayer); || prevProps.frameData !== frameData
} || prevProps.curZLayer !== curZLayer) {
if (prevProps.annotations !== annotations || prevProps.frameData !== frameData) {
this.updateCanvas(); this.updateCanvas();
} }
if (prevProps.frame !== frameData.number if (prevProps.frame !== frameData.number
&& ((resetZoom && workspace !== Workspace.ATTRIBUTE_ANNOTATION) || && ((resetZoom && workspace !== Workspace.ATTRIBUTE_ANNOTATION)
workspace === Workspace.TAG_ANNOTATION) || workspace === Workspace.TAG_ANNOTATION)
) { ) {
canvasInstance.html().addEventListener('canvas.setup', () => { canvasInstance.html().addEventListener('canvas.setup', () => {
canvasInstance.fit(); canvasInstance.fit();
@ -636,6 +632,7 @@ export default class CanvasWrapperComponent extends React.PureComponent<Props> {
private updateCanvas(): void { private updateCanvas(): void {
const { const {
curZLayer,
annotations, annotations,
frameData, frameData,
canvasInstance, canvasInstance,
@ -643,7 +640,7 @@ export default class CanvasWrapperComponent extends React.PureComponent<Props> {
if (frameData !== null) { if (frameData !== null) {
canvasInstance.setup(frameData, annotations canvasInstance.setup(frameData, annotations
.filter((e) => e.objectType !== ObjectType.TAG)); .filter((e) => e.objectType !== ObjectType.TAG), curZLayer);
} }
} }

@ -164,7 +164,7 @@ function ItemAttributeComponent(props: Props): JSX.Element {
onChange={(event: React.ChangeEvent<HTMLInputElement>): void => { onChange={(event: React.ChangeEvent<HTMLInputElement>): void => {
changeAttribute(attrID, event.target.value); changeAttribute(attrID, event.target.value);
}} }}
value={attrValue} defaultValue={attrValue}
className='cvat-object-item-text-attribute' className='cvat-object-item-text-attribute'
/> />
</Col> </Col>

@ -7,7 +7,7 @@ import { withRouter } from 'react-router-dom';
import { RouteComponentProps } from 'react-router'; import { RouteComponentProps } from 'react-router';
import AnnotationPageComponent from 'components/annotation-page/annotation-page'; 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'; import { CombinedState, Workspace } from 'reducers/interfaces';
@ -25,6 +25,7 @@ interface StateToProps {
interface DispatchToProps { interface DispatchToProps {
getJob(): void; getJob(): void;
saveLogs(): void; saveLogs(): void;
closeJob(): void;
} }
function mapStateToProps(state: CombinedState, own: OwnProps): StateToProps { function mapStateToProps(state: CombinedState, own: OwnProps): StateToProps {
@ -83,6 +84,9 @@ function mapDispatchToProps(dispatch: any, own: OwnProps): DispatchToProps {
saveLogs(): void { saveLogs(): void {
dispatch(saveLogsAsync()); dispatch(saveLogsAsync());
}, },
closeJob(): void {
dispatch(closeJobAction());
},
}; };
} }

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

@ -20,7 +20,7 @@ export function createAction<T extends string, P>(
export type ActionUnion<A extends ActionCreatorsMapObject> = ReturnType<A[keyof A]>; 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>; = _ThunkAction<R, CombinedState, {}, A>;
export type ThunkDispatch<E = {}, A extends Action = AnyAction> export type ThunkDispatch<E = {}, A extends Action = AnyAction>

Loading…
Cancel
Save