Skip to content

Commit

Permalink
we don't need to round-trip through core to reposition notes
Browse files Browse the repository at this point in the history 
we know where they are by cell addresses, so defer getting
the screen positions until we need them and we don't need
to get core to trigger recalculating them. When we redraw
the comments after the new geometry arrives then we can place
them via the address.

When adding a new note we want to know the range of the
potentially merged cells we are inserting into.

#7334

https://gerrit.libreoffice.org/c/core/+/158560 needs to be
applied to solve the problem described there that becomes
apparent when this is in place.

Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: I3228dc8fa8d47ba4e796e50427c125d7f78fe5fc
  • Loading branch information
@caolanm
caolanm committed Nov 2, 2023
1 parent 46c1248 commit 72b2ce4
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 36 deletions.
26 changes: 17 additions & 9 deletions browser/src/layer/tile/CalcTileLayer.js
View file
Expand Up @@ -21,18 +21,27 @@ L.CalcTileLayer = L.CanvasTileLayer.extend({
var commentList = app.sectionContainer.getSectionWithName(L.CSections.CommentList.name).sectionProperties.commentList;
var comment = null;

var cursorAddress = this._cellCursorXY;

for (var i = 0; i < commentList.length; i++) {
if (this._cellCursorTwips.contains(commentList[i].sectionProperties.data.cellPos)) {
if (commentList[i].sectionProperties.data.tab == this._selectedPart) {
if (commentList[i].sectionProperties.data.tab == this._selectedPart) {
if (commentList[i].sectionProperties.data.cellRange.contains(cursorAddress)) {
comment = commentList[i];
break;
}
}
}

if (!comment) {
var pixelStart = new L.Point(Math.ceil(this._cellCursorPixels.getX1()),
Math.ceil(this._cellCursorPixels.getY1()));
var rangeStart = this.sheetGeometry.getCellFromPos(pixelStart, 'corepixels');
var pixelEnd = new L.Point(Math.floor(this._cellCursorPixels.getX2() - 1),
Math.floor(this._cellCursorPixels.getY2() - 1));
var rangeEnd = this.sheetGeometry.getCellFromPos(pixelEnd, 'corepixels');

var newComment = {
cellPos: app.file.calc.cellCursor.rectangle.twips.slice(), // Copy the array.
cellRange: new L.Bounds(rangeStart, rangeEnd),
anchorPos: app.file.calc.cellCursor.rectangle.twips.slice(), // Copy the array.
id: 'new',
tab: this._selectedPart,
Expand Down Expand Up @@ -119,15 +128,12 @@ L.CalcTileLayer = L.CanvasTileLayer.extend({
if (textMsg.startsWith('invalidateheader: column')) {
this.refreshViewData({x: this._map._getTopLeftPoint().x, y: 0,
offset: {x: undefined, y: 0}}, true /* compatDataSrcOnly */);
app.socket.sendMessage('commandvalues command=.uno:ViewAnnotationsPosition');
} else if (textMsg.startsWith('invalidateheader: row')) {
this.refreshViewData({x: 0, y: this._map._getTopLeftPoint().y,
offset: {x: 0, y: undefined}}, true /* compatDataSrcOnly */);
app.socket.sendMessage('commandvalues command=.uno:ViewAnnotationsPosition');
} else if (textMsg.startsWith('invalidateheader: all')) {
this.refreshViewData({x: this._map._getTopLeftPoint().x, y: this._map._getTopLeftPoint().y,
offset: {x: undefined, y: undefined}}, true /* compatDataSrcOnly */);
app.socket.sendMessage('commandvalues command=.uno:ViewAnnotationsPosition');
} else if (this.options.sheetGeometryDataEnabled &&
textMsg.startsWith('invalidatesheetgeometry:')) {
var params = textMsg.substring('invalidatesheetgeometry:'.length).trim().split(' ');
Expand Down Expand Up @@ -246,7 +252,6 @@ L.CalcTileLayer = L.CanvasTileLayer.extend({
this._map.fire('zoomchanged');
this.refreshViewData();
this._replayPrintTwipsMsgs(false);
app.socket.sendMessage('commandvalues command=.uno:ViewAnnotationsPosition');
},

_restrictDocumentSize: function () {
Expand Down Expand Up @@ -929,8 +934,11 @@ L.CalcTileLayer = L.CanvasTileLayer.extend({
break;
}
}
if (commentObject)
commentObject.sectionProperties.data.cellPos = this._cellRangeToTwipRect(comment.cellRange).toRectangle();
if (commentObject) {
// turn cell range string into Bounds
commentObject.sectionProperties.data.cellRange = this._parseCellRange(comment.cellRange);

}
}
}

Expand Down
20 changes: 11 additions & 9 deletions browser/src/layer/tile/CanvasTileLayer.js
View file
Expand Up @@ -1565,15 +1565,21 @@ L.CanvasTileLayer = L.Layer.extend({
}
},

_cellRangeToTwipRect: function(cellRange) {
_parseCellRange: function(cellRange) {
var strTwips = cellRange.match(/\d+/g);
var startCellAddress = [parseInt(strTwips[0]), parseInt(strTwips[1])];
var startCellRectPixel = this.sheetGeometry.getCellRect(startCellAddress[0], startCellAddress[1]);
var topLeftTwips = this._corePixelsToTwips(startCellRectPixel.min);
var endCellAddress = [parseInt(strTwips[2]), parseInt(strTwips[3])];
var endCellRectPixel = this.sheetGeometry.getCellRect(endCellAddress[0], endCellAddress[1]);
return new L.Bounds(startCellAddress, endCellAddress);
},

_cellRangeToTwipRect: function(cellRange) {
var startCell = cellRange.getTopLeft();
var startCellRectPixel = this.sheetGeometry.getCellRect(startCell.x, startCell.y);
var topLeftTwips = this._corePixelsToTwips(startCellRectPixel.min);
var endCell = cellRange.getBottomRight();
var endCellRectPixel = this.sheetGeometry.getCellRect(endCell.x, endCell.y);
var bottomRightTwips = this._corePixelsToTwips(endCellRectPixel.max);
return new L.Bounds(new L.Point(topLeftTwips.x, topLeftTwips.y), new L.Point(bottomRightTwips.x, bottomRightTwips.y));
return new L.Bounds(topLeftTwips, bottomRightTwips);
},

_onMessage: function (textMsg, img) {
Expand Down Expand Up @@ -1831,10 +1837,6 @@ L.CanvasTileLayer = L.Layer.extend({
}
else if (textMsg.startsWith('comment:')) {
var obj = JSON.parse(textMsg.substring('comment:'.length + 1));
if (obj.comment.cellRange) {
// convert cellRange e.g. "A1 B2" to its bounds in display twips.
obj.comment.cellPos = this._cellRangeToTwipRect(obj.comment.cellRange).toCoreString();
}
app.sectionContainer.getSectionWithName(L.CSections.CommentList.name).onACKComment(obj);
}
else if (textMsg.startsWith('redlinetablemodified:')) {
Expand Down
21 changes: 10 additions & 11 deletions browser/src/layer/tile/CommentListSection.ts
View file
Expand Up @@ -1338,9 +1338,16 @@ export class CommentSection extends CanvasSectionObject {
// So we can use it directly.
private adjustCommentNormal (comment: any): void {
comment.trackchange = false;
comment.rectangles = this.stringToRectangles(comment.textRange || comment.anchorPos || comment.rectangle || comment.cellPos); // Simple array of point arrays [x1, y1, x2, y2].
comment.rectanglesOriginal = this.stringToRectangles(comment.textRange || comment.anchorPos || comment.rectangle || comment.cellPos); // This unmodified version will be kept for re-calculations.
comment.anchorPos = this.stringToRectangles(comment.anchorPos || comment.rectangle || comment.cellPos)[0];

if (comment.cellRange) {
// turn cell range string into cell bounds
comment.cellRange = this.sectionProperties.docLayer._parseCellRange(comment.cellRange);
}

var cellPos = comment.cellRange ? this.sectionProperties.docLayer._cellRangeToTwipRect(comment.cellRange).toRectangle() : null;
comment.rectangles = this.stringToRectangles(comment.textRange || comment.anchorPos || comment.rectangle || cellPos); // Simple array of point arrays [x1, y1, x2, y2].
comment.rectanglesOriginal = this.stringToRectangles(comment.textRange || comment.anchorPos || comment.rectangle || cellPos); // This unmodified version will be kept for re-calculations.
comment.anchorPos = this.stringToRectangles(comment.anchorPos || comment.rectangle || cellPos)[0];
comment.anchorPix = this.numberArrayToCorePixFromTwips(comment.anchorPos, 0, 2);
comment.parthash = comment.parthash ? comment.parthash: null;
comment.tab = (comment.tab || comment.tab === 0) ? comment.tab: null;
Expand All @@ -1350,9 +1357,6 @@ export class CommentSection extends CanvasSectionObject {
if (comment.rectangle) {
comment.rectangle = this.stringToRectangles(comment.rectangle)[0]; // This is the position of the marker (Impress & Draw).
}
else if (comment.cellPos) {
comment.cellPos = this.stringToRectangles(comment.cellPos)[0]; // Calc.
}

var viewId = this.map.getViewId(comment.author);
var color = viewId >= 0 ? L.LOUtil.rgbToHex(this.map.getViewColor(viewId)) : '#43ACE8';
Expand Down Expand Up @@ -1795,11 +1799,6 @@ export class CommentSection extends CanvasSectionObject {
for (var i = 0; i < commentList.length; i++) {
comment = commentList[i];

if (comment.cellRange) {
// convert cellRange e.g. "A1 B2" to its bounds in display twips.
comment.cellPos = this.sectionProperties.docLayer._cellRangeToTwipRect(comment.cellRange).toCoreString();
}

this.adjustComment(comment);
if (comment.author in this.map._viewInfoByUserName) {
comment.avatar = this.map._viewInfoByUserName[comment.author].userextrainfo.avatar;
Expand Down
18 changes: 11 additions & 7 deletions browser/src/layer/tile/CommentSection.ts
View file
Expand Up @@ -380,16 +380,17 @@ export class Comment extends CanvasSectionObject {
if (this.size[0] < 5)
this.size[0] = 5;
}
else if (this.sectionProperties.data.cellPos && this.sectionProperties.docLayer._docType === 'spreadsheet') {
else if (this.sectionProperties.data.cellRange && this.sectionProperties.docLayer._docType === 'spreadsheet') {
var ratio: number = (app.tile.size.pixels[0] / app.tile.size.twips[0]);
this.size = this.calcCellSize();
let startX = this.sectionProperties.data.cellPos[0];
var cellPos = this.map._docLayer._cellRangeToTwipRect(this.sectionProperties.data.cellRange).toRectangle();
let startX = cellPos[0];
if (this.isCalcRTL()) { // Mirroring is done in setPosition
const sizeX = this.sectionProperties.data.cellPos[2];
const sizeX = cellPos[2];
startX += sizeX; // but adjust for width of the cell.
}
this.showSection = true;
var position: Array<number> = [Math.round(this.sectionProperties.data.cellPos[0] * ratio), Math.round(this.sectionProperties.data.cellPos[1] * ratio)];
var position: Array<number> = [Math.round(cellPos[0] * ratio), Math.round(cellPos[1] * ratio)];
var splitPosCore = {x: 0, y: 0};
if (this.map._docLayer.getSplitPanesContext())
splitPosCore = this.map._docLayer.getSplitPanesContext().getSplitPos();
Expand Down Expand Up @@ -655,7 +656,8 @@ export class Comment extends CanvasSectionObject {

if (!(<any>window).mode.isMobile()) {
var ratio: number = (app.tile.size.pixels[0] / app.tile.size.twips[0]);
var originalSize = [Math.round((this.sectionProperties.data.cellPos[2]) * ratio), Math.round((this.sectionProperties.data.cellPos[3]) * ratio)];
var cellPos = this.map._docLayer._cellRangeToTwipRect(this.sectionProperties.data.cellRange).toRectangle();
var originalSize = [Math.round((cellPos[2]) * ratio), Math.round((cellPos[3]) * ratio)];

this.sectionProperties.container.style.visibility = '';

Expand Down Expand Up @@ -1129,7 +1131,8 @@ export class Comment extends CanvasSectionObject {

public calcCellSize (): number[] {
var ratio: number = (app.tile.size.pixels[0] / app.tile.size.twips[0]);
return [Math.round((this.sectionProperties.data.cellPos[2]) * ratio), Math.round((this.sectionProperties.data.cellPos[3]) * ratio)];
var cellPos = this.map._docLayer._cellRangeToTwipRect(this.sectionProperties.data.cellRange).toRectangle();
return [Math.round((cellPos[2]) * ratio), Math.round((cellPos[3]) * ratio)];
}

public onMouseEnter (): void {
Expand All @@ -1149,7 +1152,8 @@ export class Comment extends CanvasSectionObject {

var containerWidth: number = this.sectionProperties.container.getBoundingClientRect().width;
var ratio: number = (app.tile.size.pixels[0] / app.tile.size.twips[0]);
this.size = [Math.round((this.sectionProperties.data.cellPos[2]) * ratio + containerWidth), Math.round((this.sectionProperties.data.cellPos[3]) * ratio)];
var cellPos = this.map._docLayer._cellRangeToTwipRect(this.sectionProperties.data.cellRange).toRectangle();
this.size = [Math.round((cellPos[2]) * ratio + containerWidth), Math.round((cellPos[3]) * ratio)];
this.sectionProperties.commentListSection.selectById(this.sectionProperties.data.id);
this.show();
}
Expand Down

0 comments on commit 72b2ce4

Please sign in to comment.