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.

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-UI] Critical fixes #1874

Merged
merged 10 commits into from
Jul 9, 2020
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>)

### Security
-
Expand Down
2 changes: 1 addition & 1 deletion cvat-canvas/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cvat-canvas/package.json
Original file line number Diff line number Diff line change
@@ -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": {
Expand Down
11 changes: 3 additions & 8 deletions cvat-canvas/src/typescript/canvas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
17 changes: 6 additions & 11 deletions cvat-canvas/src/typescript/canvasModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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 => {
Expand All @@ -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;
}
}
Expand Down
3 changes: 1 addition & 2 deletions cvat-canvas/src/typescript/canvasView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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();
Expand Down
4 changes: 3 additions & 1 deletion cvat-canvas/src/typescript/groupHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion cvat-ui/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cvat-ui/package.json
Original file line number Diff line number Diff line change
@@ -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": {
Expand Down
8 changes: 8 additions & 0 deletions cvat-ui/src/components/annotation-page/annotation-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -20,6 +21,7 @@ interface Props {
fetching: boolean;
getJob(): void;
saveLogs(): void;
closeJob(): void;
workspace: Workspace;
}

Expand All @@ -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');
Expand All @@ -44,6 +48,10 @@ export default function AnnotationPageComponent(props: Props): JSX.Element {
if (root) {
root.style.minHeight = '';
}

if (!history.location.pathname.includes('/jobs')) {
closeJob();
}
};
}, []);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ interface StateToProps {
normalizedKeyMap: Record<string, string>;
canvasInstance: Canvas;
canvasIsReady: boolean;
curZLayer: number;
}

interface DispatchToProps {
Expand All @@ -59,6 +60,9 @@ function mapStateToProps(state: CombinedState): StateToProps {
activatedStateID,
activatedAttributeID,
states,
zLayer: {
cur,
},
},
job: {
instance: jobInstance,
Expand All @@ -85,6 +89,7 @@ function mapStateToProps(state: CombinedState): StateToProps {
normalizedKeyMap,
canvasInstance,
canvasIsReady,
curZLayer: cur,
};
}

Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -25,6 +25,7 @@ interface StateToProps {
interface DispatchToProps {
getJob(): void;
saveLogs(): void;
closeJob(): void;
}

function mapStateToProps(state: CombinedState, own: OwnProps): StateToProps {
Expand Down Expand Up @@ -83,6 +84,9 @@ function mapDispatchToProps(dispatch: any, own: OwnProps): DispatchToProps {
saveLogs(): void {
dispatch(saveLogsAsync());
},
closeJob(): void {
dispatch(closeJobAction());
},
};
}

Expand Down
7 changes: 0 additions & 7 deletions cvat-ui/src/containers/annotation-page/top-bar/top-bar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import {
searchAnnotationsAsync,
changeWorkspace as changeWorkspaceAction,
activateObject,
closeJob as closeJobAction,
} from 'actions/annotation-actions';
import { Canvas } from 'cvat-canvas-wrapper';

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -155,9 +153,6 @@ function mapDispatchToProps(dispatch: any): DispatchToProps {
dispatch(activateObject(null, null));
dispatch(changeWorkspaceAction(workspace));
},
closeJob(): void {
dispatch(closeJobAction());
},
};
}

Expand Down Expand Up @@ -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 => {
Expand Down
2 changes: 1 addition & 1 deletion cvat-ui/src/utils/redux.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down