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

Hide mask during editing #8554

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
6 changes: 6 additions & 0 deletions cvat-canvas/src/typescript/canvasModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export interface Configuration {
controlPointsSize?: number;
outlinedBorders?: string | false;
resetZoom?: boolean;
hideEditedObject?: boolean;
}

export interface BrushTool {
Expand Down Expand Up @@ -416,6 +417,7 @@ export class CanvasModelImpl extends MasterImpl implements CanvasModel {
textPosition: consts.DEFAULT_SHAPE_TEXT_POSITION,
textContent: consts.DEFAULT_SHAPE_TEXT_CONTENT,
undefinedAttrValue: consts.DEFAULT_UNDEFINED_ATTR_VALUE,
hideEditedObject: false,
},
imageBitmap: false,
image: null,
Expand Down Expand Up @@ -981,6 +983,10 @@ export class CanvasModelImpl extends MasterImpl implements CanvasModel {
this.data.configuration.CSSImageFilter = configuration.CSSImageFilter;
}

if (typeof configuration.hideEditedObject === 'boolean') {
this.data.configuration.hideEditedObject = configuration.hideEditedObject;
}

this.notify(UpdateReasons.CONFIG_UPDATED);
}

Expand Down
63 changes: 48 additions & 15 deletions cvat-canvas/src/typescript/drawHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import * as SVG from 'svg.js';
import 'svg.draw.js';
import './svg.patch';
import { CIRCLE_STROKE } from './svg.patch';

import { AutoborderHandler } from './autoborderHandler';
import {
Expand Down Expand Up @@ -104,6 +104,7 @@
private controlPointsSize: number;
private selectedShapeOpacity: number;
private outlinedBorders: string;
private isHidden: boolean;

// we should use any instead of SVG.Shape because svg plugins cannot change declared interface
// so, methods like draw() just undefined for SVG.Shape, but nevertheless they exist
Expand Down Expand Up @@ -579,7 +580,10 @@

this.drawInstance.on('drawstart', sizeDecrement);
this.drawInstance.on('drawpoint', sizeDecrement);
this.drawInstance.on('drawupdate', (): void => this.transform(this.geometry));
this.drawInstance.on('drawupdate', (): void => {
this.transform(this.geometry);
this.updateInnerCircles(!this.isHidden);
});
this.drawInstance.on('undopoint', (): number => size++);

// Add ability to cancel the latest drawn point
Expand Down Expand Up @@ -1276,6 +1280,7 @@
this.selectedShapeOpacity = configuration.selectedShapeOpacity;
this.outlinedBorders = configuration.outlinedBorders || 'black';
this.autobordersEnabled = false;
this.isHidden = false;
this.startTimestamp = Date.now();
this.onDrawDoneDefault = onDrawDone;
this.canvas = canvas;
Expand All @@ -1301,11 +1306,17 @@
});
}

public configurate(configuration: Configuration): void {
this.controlPointsSize = configuration.controlPointsSize;
this.selectedShapeOpacity = configuration.selectedShapeOpacity;
this.outlinedBorders = configuration.outlinedBorders || 'black';
private updateInnerCircles(shown: boolean) {

Check warning on line 1309 in cvat-canvas/src/typescript/drawHandler.ts

View workflow job for this annotation

GitHub Actions / Linter

Missing return type on function
const paintHandler = this.drawInstance.remember('_paintHandler');
if (paintHandler) {
for (const point of (paintHandler as any).set.members) {
point.attr('stroke', shown ? CIRCLE_STROKE : 'none');
point.fill({ opacity: shown ? 1 : 0 });
}
}
}

private updateDrawInstance(opacity: number) {

Check warning on line 1319 in cvat-canvas/src/typescript/drawHandler.ts

View workflow job for this annotation

GitHub Actions / Linter

Missing return type on function
const isFillableRect = this.drawData &&
this.drawData.shapeType === 'rectangle' &&
(this.drawData.rectDrawingMethod === RectDrawingMethod.CLASSIC || this.drawData.initialState);
Expand All @@ -1315,23 +1326,45 @@
const isFilalblePolygon = this.drawData && this.drawData.shapeType === 'polygon';

if (this.drawInstance && (isFillableRect || isFillableCuboid || isFilalblePolygon)) {
this.drawInstance.fill({ opacity: configuration.selectedShapeOpacity });
this.drawInstance.fill({ opacity });
}

if (this.drawInstance && this.drawInstance.attr('stroke')) {
this.drawInstance.attr('stroke', this.outlinedBorders);
this.drawInstance.attr('stroke', opacity ? this.outlinedBorders : 'none');
}

if (this.drawInstance && (isFilalblePolygon)) {
this.updateInnerCircles(!this.isHidden);
}

if (this.pointsGroup && this.pointsGroup.attr('stroke')) {
this.pointsGroup.attr('stroke', this.outlinedBorders);
this.pointsGroup.attr('stroke', opacity ? this.outlinedBorders : 'none');
}
}

this.autobordersEnabled = configuration.autoborders;
if (this.drawInstance && !this.drawData.initialState) {
if (this.autobordersEnabled) {
this.autoborderHandler.autoborder(true, this.drawInstance, this.drawData.redraw);
} else {
this.autoborderHandler.autoborder(false);
private updateHidden(value: boolean) {

Check warning on line 1345 in cvat-canvas/src/typescript/drawHandler.ts

View workflow job for this annotation

GitHub Actions / Linter

Missing return type on function
this.isHidden = value;
if (value) {
this.updateDrawInstance(0);
}
}

public configurate(configuration: Configuration): void {
this.controlPointsSize = configuration.controlPointsSize;
this.selectedShapeOpacity = configuration.selectedShapeOpacity;
this.outlinedBorders = configuration.outlinedBorders || 'black';

if (this.isHidden !== configuration.hideEditedObject) {
this.updateHidden(configuration.hideEditedObject);
} else if (!this.isHidden) {
this.updateDrawInstance(configuration.selectedShapeOpacity);
this.autobordersEnabled = configuration.autoborders;
if (this.drawInstance && !this.drawData.initialState) {
if (this.autobordersEnabled) {
this.autoborderHandler.autoborder(true, this.drawInstance, this.drawData.redraw);
} else {
this.autoborderHandler.autoborder(false);
}
klakhov marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down
43 changes: 36 additions & 7 deletions cvat-canvas/src/typescript/masksHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import debounce from 'lodash/debounce';

import {
DrawData, MasksEditData, Geometry, Configuration, BrushTool, ColorBy,
DrawData, MasksEditData, Geometry, Configuration, BrushTool, ColorBy, Position,
} from './canvasModel';
import consts from './consts';
import { DrawHandler } from './drawHandler';
Expand Down Expand Up @@ -61,10 +61,11 @@
private editData: MasksEditData | null;

private colorBy: ColorBy;
private latestMousePos: { x: number; y: number; };
private latestMousePos: Position;
private startTimestamp: number;
private geometry: Geometry;
private drawingOpacity: number;
private isHidden: boolean;

private keepDrawnPolygon(): void {
const canvasWrapper = this.canvas.getElement().parentElement;
Expand Down Expand Up @@ -217,12 +218,29 @@
private imageDataFromCanvas(wrappingBBox: WrappingBBox): Uint8ClampedArray {
const imageData = this.canvas.toCanvasElement()
.getContext('2d').getImageData(
wrappingBBox.left, wrappingBBox.top,
wrappingBBox.right - wrappingBBox.left + 1, wrappingBBox.bottom - wrappingBBox.top + 1,
wrappingBBox.left,
wrappingBBox.top,
wrappingBBox.right - wrappingBBox.left + 1,
wrappingBBox.bottom - wrappingBBox.top + 1,
).data;
return imageData;
}

private updateHidden(value: boolean) {

Check warning on line 229 in cvat-canvas/src/typescript/masksHandler.ts

View workflow job for this annotation

GitHub Actions / Linter

Missing return type on function
this.isHidden = value;
// Need to update style of upper canvas explicilty because update of default cursor is not applied immideately
// https://github.com/fabricjs/fabric.js/issues/1456
if (value) {
this.canvas.getElement().parentElement.style.opacity = '0';
(this.canvas.getElement().parentElement.querySelector('.upper-canvas') as HTMLElement).style.cursor = 'inherit';
this.canvas.defaultCursor = 'inherit';
} else {
this.canvas.getElement().parentElement.style.opacity = '';
(this.canvas.getElement().parentElement.querySelector('.upper-canvas') as HTMLElement).style.cursor = 'none';
this.canvas.defaultCursor = 'none';
}
}
klakhov marked this conversation as resolved.
Show resolved Hide resolved

private updateBrushTools(brushTool?: BrushTool, opts: Partial<BrushTool> = {}): void {
if (this.isPolygonDrawing) {
// tool was switched from polygon to brush for example
Expand Down Expand Up @@ -350,6 +368,7 @@
this.editData = null;
this.drawingOpacity = 0.5;
this.brushMarker = null;
this.isHidden = false;
this.colorBy = ColorBy.LABEL;
this.onDrawDone = onDrawDone;
this.onDrawRepeat = onDrawRepeat;
Expand Down Expand Up @@ -452,7 +471,7 @@
this.canvas.renderAll();
}

if (isMouseDown && !isBrushSizeChanging && ['brush', 'eraser'].includes(tool?.type)) {
if (isMouseDown && !this.isHidden && !isBrushSizeChanging && ['brush', 'eraser'].includes(tool?.type)) {
const color = fabric.Color.fromHex(tool.color);
color.setAlpha(tool.type === 'eraser' ? 1 : 0.5);

Expand Down Expand Up @@ -530,6 +549,10 @@

public configurate(configuration: Configuration): void {
this.colorBy = configuration.colorBy;

if (this.isHidden !== configuration.hideEditedObject) {
this.updateHidden(configuration.hideEditedObject);
}
}

public transform(geometry: Geometry): void {
Expand Down Expand Up @@ -563,7 +586,10 @@
const color = fabric.Color.fromHex(this.getStateColor(drawData.initialState)).getSource();
const [left, top, right, bottom] = points.slice(-4);
const imageBitmap = expandChannels(color[0], color[1], color[2], points);
imageDataToDataURL(imageBitmap, right - left + 1, bottom - top + 1,
imageDataToDataURL(
imageBitmap,
right - left + 1,
bottom - top + 1,
(dataURL: string) => new Promise((resolve) => {
fabric.Image.fromURL(dataURL, (image: fabric.Image) => {
try {
Expand Down Expand Up @@ -654,7 +680,10 @@
const color = fabric.Color.fromHex(this.getStateColor(editData.state)).getSource();
const [left, top, right, bottom] = points.slice(-4);
const imageBitmap = expandChannels(color[0], color[1], color[2], points);
imageDataToDataURL(imageBitmap, right - left + 1, bottom - top + 1,
imageDataToDataURL(
imageBitmap,
right - left + 1,
bottom - top + 1,
(dataURL: string) => new Promise((resolve) => {
fabric.Image.fromURL(dataURL, (image: fabric.Image) => {
try {
Expand Down
1 change: 1 addition & 0 deletions cvat-canvas/src/typescript/svg.patch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ SVG.Element.prototype.draw.extend(
}),
);

export const CIRCLE_STROKE = '#000';
klakhov marked this conversation as resolved.
Show resolved Hide resolved
// Fix method drawCircles
function drawCircles(): void {
const array = this.el.array().valueOf();
Expand Down
83 changes: 83 additions & 0 deletions cvat-ui/src/actions/annotation-actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,9 @@ export enum AnnotationActionTypes {
COLLAPSE_APPEARANCE = 'COLLAPSE_APPEARANCE',
COLLAPSE_OBJECT_ITEMS = 'COLLAPSE_OBJECT_ITEMS',
ACTIVATE_OBJECT = 'ACTIVATE_OBJECT',
UPDATE_EDITED_STATE = 'UPDATE_EDITED_STATE',
HIDE_EDITED_STATE = 'HIDE_EDITED_STATE',
RESET_EDITED_STATE = 'RESET_EDITED_STATE',
REMOVE_OBJECT = 'REMOVE_OBJECT',
REMOVE_OBJECT_SUCCESS = 'REMOVE_OBJECT_SUCCESS',
REMOVE_OBJECT_FAILED = 'REMOVE_OBJECT_FAILED',
Expand Down Expand Up @@ -1608,3 +1611,83 @@ export function restoreFrameAsync(frame: number): ThunkAction {
}
};
}

export function updateEditedStateAsync(
shapeType: ShapeType | null,
editedStateInstance: ObjectState | null,
): ThunkAction {
return async (dispatch: ThunkDispatch, getState): Promise<void> => {
if (editedStateInstance) {
const state = getState();
const { instance: canvas } = state.annotation.canvas;
if (canvas) {
(canvas as Canvas).configure({
hideEditedObject: editedStateInstance.hidden,
});
}

dispatch({
type: AnnotationActionTypes.HIDE_EDITED_STATE,
payload: {
hide: editedStateInstance.hidden,
},
});
}
Comment on lines +1620 to +1635
Copy link
Member

Choose a reason for hiding this comment

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

I think we have three actions changeHideEditedStateAsync, updateEditedStateAsync, resetEditedStateAsync doing the same thing (updating the same key in store).

Probably we may combine them to reduce the code duplication. Let it accept shapeType, objectState and isHidden. And use these arguments to update store and objectState properly

Copy link
Member

Choose a reason for hiding this comment

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

Considering my general review comment it should only accept objectState: ObjectState | null

dispatch({
type: AnnotationActionTypes.UPDATE_EDITED_STATE,
payload: {
shapeType,
editedStateInstance,
},
});
};
}

export function resetEditedStateAsync(): ThunkAction {
return async (dispatch: ThunkDispatch, getState): Promise<void> => {
const state = getState();
const { instance: canvas } = state.annotation.canvas;
if (canvas) {
(canvas as Canvas).configure({
hideEditedObject: false,
});
}

dispatch({
type: AnnotationActionTypes.RESET_EDITED_STATE,
payload: {},
});
Comment on lines +1657 to +1660
Copy link
Member

Choose a reason for hiding this comment

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

dispatch(resetEditedState())?

Copy link
Member

Choose a reason for hiding this comment

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

Not really relevant considering my other comment with proposal to combine actions

};
}
klakhov marked this conversation as resolved.
Show resolved Hide resolved

export function resetEditedState(): AnyAction {
return {
type: AnnotationActionTypes.RESET_EDITED_STATE,
payload: {},
};
}

export function changeHideEditedStateAsync(hide: boolean): ThunkAction {
return async (dispatch: ThunkDispatch, getState): Promise<void> => {
const state = getState();
const { instance: canvas } = state.annotation.canvas;
if (canvas) {
(canvas as Canvas).configure({
hideEditedObject: hide,
});

dispatch({
type: AnnotationActionTypes.HIDE_EDITED_STATE,
payload: {
hide,
},
});

const { editedStateInstance } = state.annotation.annotations.editedState;
if (editedStateInstance) {
editedStateInstance.hidden = hide;
await dispatch(updateAnnotationsAsync([editedStateInstance]));
}
}
};
}
Loading
Loading