Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

Add undo and redo to annotations. #4482

Merged
merged 6 commits into from
May 29, 2018
Merged

Add undo and redo to annotations. #4482

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4f7b66c
Add undo & redo for annotations. (#4370, #4371)
chenba May 13, 2018
a50e819
Scale the drawing when saving history. (#4370)
chenba May 24, 2018
7ecd764
Scale once and disable reset button. (#4370, #4371, #4453)
chenba May 25, 2018
10b253d
Replace switch with if-else. (#4370, #4371)
chenba May 25, 2018
e1deac6
Prevent decimals from zoom or DPI scaling. (#4370, #4371)
chenba May 25, 2018
0b4a2f2
Use more CSS and less svg files. (#4370, #4371)
chenba May 25, 2018
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
4 changes: 4 additions & 0 deletions locales/en-US/server.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,10 @@ annotationPenButton =
.title = Pen
annotationHighlighterButton =
.title = Highlighter
annotationUndoButton =
.title = Undo
annotationRedoButton =
.title = Redo
# Note: This button reverts all the changes on the image since the start of the editing session.
annotationClearButton =
.title = Clear
Expand Down
12 changes: 6 additions & 6 deletions server/src/pages/shot/drawing-tool.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,14 @@ exports.DrawingTool = class DrawingTool extends React.Component {
return;
}

this.drawnArea.left = Math.max(this.drawnArea.left - this.state.lineWidth, 0);
this.drawnArea.top = Math.max(this.drawnArea.top - this.state.lineWidth, 0);
this.drawnArea.right = Math.min(
this.drawnArea.left = Math.ceil(Math.max(this.drawnArea.left - this.state.lineWidth, 0));
this.drawnArea.top = Math.ceil(Math.max(this.drawnArea.top - this.state.lineWidth, 0));
this.drawnArea.right = Math.ceil(Math.min(
this.drawnArea.right + this.state.lineWidth,
this.canvasWidth);
this.drawnArea.bottom = Math.min(
this.canvasWidth));
this.drawnArea.bottom = Math.ceil(Math.min(
this.drawnArea.bottom + this.state.lineWidth,
this.canvasHeight);
this.canvasHeight));

this.finalize();

Expand Down
118 changes: 118 additions & 0 deletions server/src/pages/shot/editor-history.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
const { Selection } = require("../../../shared/selection");

exports.EditorHistory = class {
constructor(devicePixelRatio) {
this.beforeEdits = [];
this.afterEdits = [];
this.devicePixelRatio = devicePixelRatio;
}

push(canvas, area, recordType) {
const record = new EditRecord(
canvas,
area,
this.devicePixelRatio,
recordType
);
this.beforeEdits.push(record);
this.afterEdits = [];
}

pushDiff(canvas, area) {
this.push(canvas, area, RecordType.DIFF);
}

pushFrame(canvas, area) {
this.push(canvas, area, RecordType.FRAME);
}

canUndo() {
return !!this.beforeEdits.length;
}

undo(canvasBeforeUndo) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Both undo and redo takes in the canvas before the action is applied because we need to save the image or part of it to revert the action (undo a redo or redo an undo).

if (!this.canUndo()) {
return null;
}

return this._replay(canvasBeforeUndo, this.beforeEdits, this.afterEdits);
}

canRedo() {
return !!this.afterEdits.length;
}

redo(canvasBeforeRedo) {
if (!this.canRedo()) {
return null;
}

return this._replay(canvasBeforeRedo, this.afterEdits, this.beforeEdits);
}

_replay(canvasBeforeChange, from, to) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit confused by this part of the code. If I understand correctly, in the push method above, each time the image is edited, an EditRecord is appended to this.beforeEdits. Why, then, does this method need to create a new EditRecord each time undo or redo is pressed? I wonder if it would be more performant to have just one array of edits, and, when undo or redo is pressed, increment or decrement an index cursor into that existing array of EditRecords.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It can't work like that. Or at least we can't have just one array of edits. When the user is editing the image, the affected area before the change is saved, but not the change itself. So when the user performs an undo, we need to save the change at that time so it can be (re-)applied should the user decide to redo.

Using an index is possible. I think it's a matter trading memory usage and less clean(?) code for performance at that point.

const fromRecord = from.pop();

let area = fromRecord.area;
if (fromRecord.recordType === RecordType.FRAME) {
area = new Selection(
0, 0,
parseInt(canvasBeforeChange.style.width, 10),
parseInt(canvasBeforeChange.style.height, 10)
);
}

const toRecord = new EditRecord(
canvasBeforeChange,
area,
this.devicePixelRatio,
fromRecord.recordType
);

to.push(toRecord);

return fromRecord;
}
};

class EditRecord {
constructor(canvas, area, devicePixelRatio, recordType) {
this.area = area;
this.recordType = recordType;
this.canvas = this.captureCanvas(canvas, area, devicePixelRatio, recordType);
}

captureCanvas(canvas, area, devicePixelRatio, recordType) {
const copy = document.createElement("canvas");

if (recordType === RecordType.FRAME) {
copy.width = canvas.width;
copy.height = canvas.height;
const copyContext = copy.getContext("2d");
copyContext.scale(devicePixelRatio, devicePixelRatio);
copyContext.drawImage(
canvas,
0, 0, canvas.width, canvas.height,
0, 0, area.width, area.height);
return copy;
}

copy.width = area.width * devicePixelRatio;
copy.height = area.height * devicePixelRatio;
const copyContext = copy.getContext("2d");
copyContext.scale(devicePixelRatio, devicePixelRatio);
copyContext.drawImage(
canvas,
area.left * devicePixelRatio,
area.top * devicePixelRatio,
area.width * devicePixelRatio,
area.height * devicePixelRatio,
0, 0, area.width, area.height
);

return copy;
}
}

const RecordType = { DIFF: 0, FRAME: 1 };
exports.RecordType = RecordType;
116 changes: 98 additions & 18 deletions server/src/pages/shot/editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ const { PenTool } = require("./pen-tool");
const { HighlighterTool } = require("./highlighter-tool");
const { CropTool } = require("./crop-tool");
const { ColorPicker } = require("./color-picker");
const { EditorHistory, RecordType } = require("./editor-history");
const { Selection } = require("../../../shared/selection");

exports.Editor = class Editor extends React.Component {
constructor(props) {
Expand All @@ -15,17 +17,20 @@ exports.Editor = class Editor extends React.Component {
|| props.clip.image.captureType === "fullPageTruncated") {
this.devicePixelRatio = 1;
}
this.canvasWidth = Math.floor(this.props.clip.image.dimensions.x);
this.canvasHeight = Math.floor(this.props.clip.image.dimensions.y);
this.state = {
canvasWidth: Math.floor(this.props.clip.image.dimensions.x),
canvasHeight: Math.floor(this.props.clip.image.dimensions.y),
tool: "",
color: "",
lineWidth: "",
actionsDisabled: true
actionsDisabled: true,
canUndo: false,
canRedo: false
};
this.onMouseUp = this.onMouseUp.bind(this);
this.onMouseMove = this.onMouseMove.bind(this);
this.selectedTool = React.createRef();
this.history = new EditorHistory(this.devicePixelRatio);
}

render() {
Expand All @@ -40,17 +45,19 @@ exports.Editor = class Editor extends React.Component {
}

renderCanvas(toolContent) {
const canvasWidth = Math.floor(this.state.canvasWidth * this.devicePixelRatio);
const canvasHeight = Math.floor(this.state.canvasHeight * this.devicePixelRatio);
return <div className="main-container">
<div
className={`inverse-color-scheme canvas-container ${this.state.tool}`}
id="canvas-container"
style={{ height: this.canvasHeight, width: this.canvasWidth }}>
style={{ height: this.state.canvasHeight, width: this.state.canvasWidth }}>
<canvas
className="image-holder centered"
id="image-holder"
ref={(image) => { this.imageCanvas = image; }}
height={this.canvasHeight * this.devicePixelRatio} width={this.canvasWidth * this.devicePixelRatio}
style={{ height: this.canvasHeight, width: this.canvasWidth }}></canvas>
height={canvasHeight} width={canvasWidth}
style={{ height: this.state.canvasHeight, width: this.state.canvasWidth }}></canvas>
{toolContent}
</div>
</div>;
Expand Down Expand Up @@ -91,6 +98,13 @@ exports.Editor = class Editor extends React.Component {
this.setState({overrideToolbar: this.state.tool});
}

deriveButtonStates() {
this.setState({
canUndo: this.history.canUndo(),
canRedo: this.history.canRedo()
});
}

isToolActive(tool) {
return this.state.tool === tool;
}
Expand All @@ -99,14 +113,19 @@ exports.Editor = class Editor extends React.Component {
if (this.selectedTool.current && this.selectedTool.current.renderToolbar) {
return this.selectedTool.current.renderToolbar();
}

const penState = this.isToolActive("pen") ? "active" : "inactive";
const highlighterState = this.isToolActive("highlighter") ? "active" : "inactive";
const undoButtonState = this.state.canUndo ? "active" : "inactive";
const redoButtonState = this.state.canRedo ? "active" : "inactive";

return <div className="editor-header default-color-scheme">
<div className="shot-main-actions annotation-main-actions">
<div className="annotation-tools">
<Localized id="annotationCropButton">
<button className={`button transparent crop-button`} id="crop" onClick={this.onClickCrop.bind(this)} title="Crop"></button>
</Localized>
<span className="annotation-divider"></span>
<Localized id="annotationPenButton">
<button className={`button transparent pen-button ${penState}`} id="pen" onClick={this.onClickPen.bind(this)} title="Pen"></button>
</Localized>
Expand All @@ -116,8 +135,18 @@ exports.Editor = class Editor extends React.Component {
<ColorPicker activeTool={this.state.tool}
setColorCallback={this.setColor.bind(this)}
color={this.state.color} />
<span className="annotation-divider"></span>
<Localized id="annotationUndoButton">
<button className={`button transparent undo-button ${undoButtonState}`} id="undo"
disabled={!this.state.canUndo} onClick={this.onUndo.bind(this)} title="Undo"></button>
</Localized>
<Localized id="annotationRedoButton">
<button className={`button transparent redo-button ${redoButtonState}`} id="redo"
disabled={!this.state.canRedo} onClick={this.onRedo.bind(this)} title="Redo"></button>
</Localized>
<Localized id="annotationClearButton">
<button className={`button transparent clear-button`} id="clear" onClick={this.onClickClear.bind(this)} title="Clear"></button>
<button className={`button transparent clear-button ${undoButtonState}`} id="clear"
disabled={!this.state.canUndo} onClick={this.onClickClear.bind(this)} title="Clear"></button>
</Localized>
</div>
</div>
Expand Down Expand Up @@ -155,13 +184,24 @@ exports.Editor = class Editor extends React.Component {
return;
}

this.history.pushDiff(this.imageCanvas, affectedArea);

this.imageContext.globalCompositeOperation = (compositeOp || "source-over");
this.imageContext.drawImage(incomingCanvas,
affectedArea.left * this.devicePixelRatio,
affectedArea.top * this.devicePixelRatio,
affectedArea.width * this.devicePixelRatio,
affectedArea.height * this.devicePixelRatio,
affectedArea.left, affectedArea.top, affectedArea.width, affectedArea.height);

this.deriveButtonStates();
}

applyDiff(area, diffCanvas) {
this.imageContext.globalCompositeOperation = "source-over";
this.imageContext.drawImage(diffCanvas,
Copy link
Member

Choose a reason for hiding this comment

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

I suspect this might be where the devicePixelRatio is being left out

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmmm scale() should have been called on that drawing context at that point.

Also possibly related to #4453.

0, 0, diffCanvas.width, diffCanvas.height,
area.left, area.top, area.width, area.height);
}

onCropUpdate(affectedArea, incomingCanvas) {
Expand All @@ -170,29 +210,68 @@ exports.Editor = class Editor extends React.Component {
return;
}

this.history.pushFrame(this.imageCanvas, new Selection(
0, 0, this.state.canvasWidth, this.state.canvasHeight
));
this.applyFrame(affectedArea, incomingCanvas);
this.setState({tool: this.previousTool});
this.deriveButtonStates();
}

applyFrame(area, frameCanvas) {
const img = new Image();
img.crossOrigin = "Anonymous";
img.onload = () => {
imageContext.scale(this.devicePixelRatio, this.devicePixelRatio);
imageContext.drawImage(img, 0, 0, affectedArea.width, affectedArea.height);
imageContext.drawImage(img, 0, 0, area.width, area.height);
};
this.canvasWidth = affectedArea.width;
this.canvasHeight = affectedArea.height;
const imageContext = this.imageCanvas.getContext("2d");
this.imageContext = imageContext;
img.src = incomingCanvas.toDataURL("image/png");
this.setState({tool: this.previousTool});
img.src = frameCanvas.toDataURL("image/png");
this.setState({canvasWidth: area.width, canvasHeight: area.height});
}

onClickCancelCrop() {
this.setState({tool: this.previousTool});
}

onUndo() {
if (!this.history.canUndo()) {
return;
}

this.applyHistory(this.history.undo(this.imageCanvas));
this.deriveButtonStates();
}

onRedo() {
if (!this.history.canRedo()) {
return;
}

this.applyHistory(this.history.redo(this.imageCanvas));
this.deriveButtonStates();
}

applyHistory(record) {
if (!record) {
return;
}
if (record.recordType === RecordType.DIFF) {
this.applyDiff(record.area, record.canvas);
} else {
this.applyFrame(record.area, record.canvas);
}
}

onClickClear() {
this.setState({tool: this.state.tool});
this.setState({
canvasWidth: Math.floor(this.props.clip.image.dimensions.x),
canvasHeight: Math.floor(this.props.clip.image.dimensions.y)
});
this.history = new EditorHistory(this.devicePixelRatio);
this.deriveButtonStates();
this.imageContext.clearRect(0, 0, this.imageCanvas.width, this.imageCanvas.height);
this.canvasHeight = this.props.clip.image.dimensions.y;
this.canvasWidth = this.props.clip.image.dimensions.x;
this.renderImage();
sendEvent("clear-select", "annotation-toolbar");
}
Expand All @@ -214,7 +293,7 @@ exports.Editor = class Editor extends React.Component {
}
}

const dimensions = {x: this.canvasWidth, y: this.canvasHeight};
const dimensions = {x: this.state.canvasWidth, y: this.state.canvasHeight};
this.props.onClickSave(dataUrl, dimensions);
sendEvent("save", "annotation-toolbar");
}
Expand Down Expand Up @@ -255,12 +334,11 @@ exports.Editor = class Editor extends React.Component {
}

renderImage() {
const imageContext = this.imageCanvas.getContext("2d");
imageContext.scale(this.devicePixelRatio, this.devicePixelRatio);
const img = new Image();
img.crossOrigin = "Anonymous";
const width = this.props.clip.image.dimensions.x;
const height = this.props.clip.image.dimensions.y;
const imageContext = this.imageCanvas.getContext("2d");
img.onload = () => {
imageContext.drawImage(img, 0, 0, width, height);
this.setState({actionsDisabled: false});
Expand All @@ -271,6 +349,8 @@ exports.Editor = class Editor extends React.Component {

componentDidMount() {
document.addEventListener("mouseup", this.onMouseUp);
const imageContext = this.imageCanvas.getContext("2d");
imageContext.scale(this.devicePixelRatio, this.devicePixelRatio);
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need to move the scale call into componentDidMount?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Doesn't have to be componentDidMount, it just have to execute only once. Otherwise the shot could be scaled multiple times, once per crop.

this.renderImage();
this.setState({
tool: "pen",
Expand Down
Loading