Skip to content

Commit

Permalink
React UI: Batch of fixes (#1383)
Browse files Browse the repository at this point in the history
* Fixed: cannot read property 'set' of undefined

* Fixed UI failing: save during drag/resize

* Fixed multiple saving (shortcut sticking)

* Undo/redo fixed

* Allowed one interpolated point

* Fixed API reaction when repository synchronization is failed

* Updated changelog
  • Loading branch information
bsekachev authored Apr 13, 2020
1 parent 59ec5c4 commit cf511ae
Show file tree
Hide file tree
Showing 10 changed files with 84 additions and 27 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Button to reset colors settings (brightness, saturation, contrast) in the new UI
- Option to display shape text always
- Dedicated message with clarifications when share is unmounted (https://github.com/opencv/cvat/pull/1373)
- Ability to create one tracked point (https://github.com/opencv/cvat/pull/1383)
- Tutorial: instructions for CVAT over HTTPS

### Changed
Expand All @@ -35,6 +36,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Merge is allowed for points, but clicks on points conflict with frame dragging logic
- Removed objects are visible for search
- Add missed task_id and job_id fields into exception logs for the new UI (https://github.com/opencv/cvat/pull/1372)
- UI fails when annotations saving occurs during drag/resize/edit (https://github.com/opencv/cvat/pull/1383)
- Multiple savings when hold Ctrl+S (a lot of the same copies of events were sent with the same working time) (https://github.com/opencv/cvat/pull/1383)
- UI doesn't have any reaction when git repos synchronization failed (https://github.com/opencv/cvat/pull/1383)
- Bug when annotations cannot be saved after (delete - save - undo - save) (https://github.com/opencv/cvat/pull/1383)
- VOC format exports Upper case labels correctly in lower case (https://github.com/opencv/cvat/pull/1379)
- Fixed polygon exporting bug in COCO dataset (https://github.com/opencv/cvat/issues/1387)
- Task creation from remote files (https://github.com/opencv/cvat/pull/1392)
Expand Down
4 changes: 3 additions & 1 deletion cvat-canvas/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ Standard JS events are used.
| | IDLE | GROUP | SPLIT | DRAW | MERGE | EDIT | DRAG | RESIZE | ZOOM_CANVAS | DRAG_CANVAS |
|--------------|------|-------|-------|------|-------|------|------|--------|-------------|-------------|
| html() | + | + | + | + | + | + | + | + | + | + |
| setup() | + | + | + | + | + | - | - | - | + | + |
| setup() | + | + | + | + | + | +/- | +/- | +/- | + | + |
| activate() | + | - | - | - | - | - | - | - | - | - |
| rotate() | + | + | + | + | + | + | + | + | + | + |
| focus() | + | + | + | + | + | + | + | + | + | + |
Expand All @@ -199,3 +199,5 @@ Standard JS events are used.
| configure() | + | - | - | - | - | - | - | - | - | - |
| bitmap() | + | + | + | + | + | + | + | + | + | + |
| setZLayer() | + | + | + | + | + | + | + | + | + | + |

You can call setup() during editing, dragging, and resizing only to update objects, not to change a frame.
10 changes: 8 additions & 2 deletions cvat-canvas/src/typescript/canvasModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,8 +327,10 @@ export class CanvasModelImpl extends MasterImpl implements CanvasModel {
}

public setup(frameData: any, objectStates: any[]): void {
if ([Mode.EDIT, Mode.DRAG, Mode.RESIZE].includes(this.data.mode)) {
throw Error(`Canvas is busy. Action: ${this.data.mode}`);
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) {
Expand Down Expand Up @@ -364,6 +366,10 @@ export class CanvasModelImpl extends MasterImpl implements CanvasModel {
}

public activate(clientID: number | null, attributeID: number | null): void {
if (this.data.activeElement.clientID === clientID) {
return;
}

if (this.data.mode !== Mode.IDLE && clientID !== null) {
// Exception or just return?
throw Error(`Canvas is busy. Action: ${this.data.mode}`);
Expand Down
40 changes: 22 additions & 18 deletions cvat-canvas/src/typescript/canvasView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ export class CanvasViewImpl implements CanvasView, Listener {
created.push(state);
} else {
const drawnState = this.drawnStates[state.clientID];
// object has been changed or changed frame for a track
if (drawnState.updated !== state.updated || drawnState.frame !== state.frame) {
updated.push(state);
}
Expand All @@ -395,29 +396,30 @@ export class CanvasViewImpl implements CanvasView, Listener {
.filter((id: number): boolean => !newIDs.includes(id))
.map((id: number): any => this.drawnStates[id]);

if (deleted.length || updated.length || created.length) {
if (this.activeElement.clientID !== null) {
this.deactivate();
}

if (this.activeElement.clientID !== null) {
this.deactivate();
}
for (const state of deleted) {
if (state.clientID in this.svgTexts) {
this.svgTexts[state.clientID].remove();
}

for (const state of deleted) {
if (state.clientID in this.svgTexts) {
this.svgTexts[state.clientID].remove();
this.svgShapes[state.clientID].off('click.canvas');
this.svgShapes[state.clientID].remove();
delete this.drawnStates[state.clientID];
}

this.svgShapes[state.clientID].off('click.canvas');
this.svgShapes[state.clientID].remove();
delete this.drawnStates[state.clientID];
}

this.addObjects(created, translate);
this.updateObjects(updated, translate);
this.sortObjects();
this.addObjects(created, translate);
this.updateObjects(updated, translate);
this.sortObjects();

if (this.controller.activeElement.clientID !== null) {
const { clientID } = this.controller.activeElement;
if (states.map((state: any): number => state.clientID).includes(clientID)) {
this.activate(this.controller.activeElement);
if (this.controller.activeElement.clientID !== null) {
const { clientID } = this.controller.activeElement;
if (states.map((state: any): number => state.clientID).includes(clientID)) {
this.activate(this.controller.activeElement);
}
}
}
}
Expand Down Expand Up @@ -960,6 +962,8 @@ export class CanvasViewImpl implements CanvasView, Listener {
attributes: { ...state.attributes },
zOrder: state.zOrder,
pinned: state.pinned,
updated: state.updated,
frame: state.frame,
};
}

Expand Down
4 changes: 3 additions & 1 deletion cvat-canvas/src/typescript/svg.patch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ SVG.Element.prototype.draw = function constructor(...args: any): any {
if (!handler) {
originalDraw.call(this, ...args);
handler = this.remember('_paintHandler');
if (!handler.set) {
// There is use case (drawing a single point when handler is created and destructed immediately in one stack)
// So, we need to check if handler still exists
if (handler && !handler.set) {
handler.set = new SVG.Set();
}
} else {
Expand Down
27 changes: 27 additions & 0 deletions cvat-core/src/annotations-objects.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,10 @@

this.history.do(HistoryActions.CHANGED_LOCK, () => {
this.lock = undoLock;
this.updated = Date.now();
}, () => {
this.lock = redoLock;
this.updated = Date.now();
}, [this.clientID], frame);

this.lock = lock;
Expand All @@ -197,8 +199,10 @@

this.history.do(HistoryActions.CHANGED_COLOR, () => {
this.color = undoColor;
this.updated = Date.now();
}, () => {
this.color = redoColor;
this.updated = Date.now();
}, [this.clientID], frame);

this.color = color;
Expand All @@ -210,8 +214,10 @@

this.history.do(HistoryActions.CHANGED_HIDDEN, () => {
this.hidden = undoHidden;
this.updated = Date.now();
}, () => {
this.hidden = redoHidden;
this.updated = Date.now();
}, [this.clientID], frame);

this.hidden = hidden;
Expand All @@ -229,9 +235,11 @@
this.history.do(HistoryActions.CHANGED_LABEL, () => {
this.label = undoLabel;
this.attributes = undoAttributes;
this.updated = Date.now();
}, () => {
this.label = redoLabel;
this.attributes = redoAttributes;
this.updated = Date.now();
}, [this.clientID], frame);
}

Expand All @@ -246,8 +254,10 @@

this.history.do(HistoryActions.CHANGED_ATTRIBUTES, () => {
this.attributes = undoAttributes;
this.updated = Date.now();
}, () => {
this.attributes = redoAttributes;
this.updated = Date.now();
}, [this.clientID], frame);
}

Expand Down Expand Up @@ -373,9 +383,12 @@
this.removed = true;

this.history.do(HistoryActions.REMOVED_OBJECT, () => {
this.serverID = undefined;
this.removed = false;
this.updated = Date.now();
}, () => {
this.removed = true;
this.updated = Date.now();
}, [this.clientID], frame);
}

Expand All @@ -398,8 +411,10 @@

this.history.do(HistoryActions.CHANGED_PINNED, () => {
this.pinned = undoPinned;
this.updated = Date.now();
}, () => {
this.pinned = redoPinned;
this.updated = Date.now();
}, [this.clientID], frame);

this.pinned = pinned;
Expand Down Expand Up @@ -489,8 +504,10 @@

this.history.do(HistoryActions.CHANGED_POINTS, () => {
this.points = undoPoints;
this.updated = Date.now();
}, () => {
this.points = redoPoints;
this.updated = Date.now();
}, [this.clientID], frame);

this.points = points;
Expand All @@ -502,8 +519,10 @@

this.history.do(HistoryActions.CHANGED_OCCLUDED, () => {
this.occluded = undoOccluded;
this.updated = Date.now();
}, () => {
this.occluded = redoOccluded;
this.updated = Date.now();
}, [this.clientID], frame);

this.occluded = occluded;
Expand All @@ -515,8 +534,10 @@

this.history.do(HistoryActions.CHANGED_ZORDER, () => {
this.zOrder = undoZOrder;
this.updated = Date.now();
}, () => {
this.zOrder = redoZOrder;
this.updated = Date.now();
}, [this.clientID], frame);

this.zOrder = zOrder;
Expand Down Expand Up @@ -777,12 +798,14 @@
for (const mutable of undoAttributes.mutable) {
this.shapes[mutable.frame].attributes = mutable.attributes;
}
this.updated = Date.now();
}, () => {
this.label = redoLabel;
this.attributes = redoAttributes.unmutable;
for (const mutable of redoAttributes.mutable) {
this.shapes[mutable.frame].attributes = mutable.attributes;
}
this.updated = Date.now();
}, [this.clientID], frame);
}

Expand Down Expand Up @@ -853,11 +876,13 @@
} else if (redoShape) {
delete this.shapes[frame];
}
this.updated = Date.now();
}, () => {
this.attributes = redoAttributes;
if (redoShape) {
this.shapes[frame] = redoShape;
}
this.updated = Date.now();
}, [this.clientID], frame);
}

Expand All @@ -868,12 +893,14 @@
} else {
this.shapes[frame] = undoShape;
}
this.updated = Date.now();
}, () => {
if (!redoShape) {
delete this.shapes[frame];
} else {
this.shapes[frame] = redoShape;
}
this.updated = Date.now();
}, [this.clientID], frame);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ function DrawShapePopoverComponent(props: Props): JSX.Element {
onChangeRectDrawingMethod,
} = props;

const trackDisabled = shapeType === ShapeType.POLYGON || shapeType === ShapeType.POLYLINE
|| (shapeType === ShapeType.POINTS && numberOfPoints !== 1);

return (
<div className='cvat-draw-shape-popover-content'>
<Row type='flex' justify='start'>
Expand Down Expand Up @@ -148,7 +151,7 @@ function DrawShapePopoverComponent(props: Props): JSX.Element {
<Tooltip title={`Press ${repeatShapeShortcut} to draw again`}>
<Button
onClick={onDrawTrack}
disabled={shapeType !== ShapeType.RECTANGLE}
disabled={trackDisabled}
>
Track
</Button>
Expand Down
8 changes: 7 additions & 1 deletion cvat-ui/src/components/task-page/details.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -299,8 +299,14 @@ export default class DetailsComponent extends React.PureComponent<Props, State>
repositoryStatus: 'sync',
});
}
}).catch((): void => {
}).catch((error): void => {
if (this.mounted) {
Modal.error({
width: 800,
title: 'Could not synchronize the repository',
content: error.toString(),
});

this.setState({
repositoryStatus: '!sync',
});
Expand Down
4 changes: 3 additions & 1 deletion cvat-ui/src/containers/annotation-page/top-bar/top-bar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,9 @@ class AnnotationTopBarContainer extends React.PureComponent<Props> {
},
SAVE_JOB: (event: KeyboardEvent | undefined) => {
preventDefault(event);
this.onSaveAnnotation();
if (!saving) {
this.onSaveAnnotation();
}
},
NEXT_FRAME: (event: KeyboardEvent | undefined) => {
preventDefault(event);
Expand Down
4 changes: 2 additions & 2 deletions cvat-ui/src/utils/git-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,10 @@ export function syncRepos(tid: number): Promise<void> {
resolve();
} else if (response.status === 'failed') {
const message = `Can not push to remote repository. Message: ${response.stderr}`;
throw new Error(message);
reject(new Error(message));
} else {
const message = `Check returned status "${response.status}".`;
throw new Error(message);
reject(new Error(message));
}
}

Expand Down

0 comments on commit cf511ae

Please sign in to comment.