Skip to content

Commit

Permalink
Render crisp lines/strokes (#307)
Browse files Browse the repository at this point in the history 
* Render crisp lines/strokes

The middle of lines/strokes on canvas is positioned exactly in between two pixels. This is explained here https://developer.mozilla.org/en-US/docs/Web/API/Canvas_API/Tutorial/Applying_styles_and_colors#A_lineWidth_example

This leads to blurry and too thick lines. 1 px solid black grid lines are displayed as 2 px lines in medium grey. This is somewhat mitigated when lines overlay each other.

The fix attempts to shift all canvas coordinates by half a pixel via the `canvasOffsetTop` and `canvasOffsetLeft` properties.

* Adjust tests to crisper line rendering

* Fix blurry rendering of images after rendering crips lines now

drawHtml() positions images now on half pixels after the crips lines change. But this renders images blurry.
This patch rounds the coordinates to the nearest full pixel.

* Fix drawActiveCell() is shifted by a pixel in selectionMode: row
  • Loading branch information
@tmaroschik
tmaroschik committed Sep 25, 2020
1 parent b9313e3 commit f70ba1b
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 26 deletions.
10 changes: 5 additions & 5 deletions lib/draw.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ define([], function () {
var img,
v = cell.innerHTML || cell.formattedValue,
cacheKey = v.toString() + cell.rowIndex.toString() + cell.columnIndex.toString(),
x = cell.x + self.canvasOffsetLeft,
y = cell.y + self.canvasOffsetTop;
x = Math.round(cell.x + self.canvasOffsetLeft),
y = Math.round(cell.y + self.canvasOffsetTop);
if (self.htmlImageCache[cacheKey]) {
img = self.htmlImageCache[cacheKey].img;
if (self.htmlImageCache[cacheKey].height !== cell.height || self.htmlImageCache[cacheKey].width !== cell.width) {
Expand Down Expand Up @@ -1038,8 +1038,8 @@ define([], function () {
self.visibleRows = [];
s = self.getSchema();
self.visibleCells = [];
self.canvasOffsetTop = self.isChildGrid ? self.parentNode.offsetTop : 0;
self.canvasOffsetLeft = self.isChildGrid ? self.parentNode.offsetLeft : 0;
self.canvasOffsetTop = self.isChildGrid ? self.parentNode.offsetTop : .5;
self.canvasOffsetLeft = self.isChildGrid ? self.parentNode.offsetLeft : -.5;
h = self.height;
w = self.width;
}
Expand Down Expand Up @@ -1229,7 +1229,7 @@ define([], function () {
if (self.activeCell && self.activeCell.rowIndex === aCell.rowIndex) {
self.ctx.lineWidth = self.style.activeCellOverlayBorderWidth;
self.ctx.strokeStyle = self.style.activeCellOverlayBorderColor;
strokeRect(0, aCell.y, self.getHeaderWidth() + rowHeaderCellWidth, self.visibleRowHeights[aCell.rowIndex]);
strokeRect(1, aCell.y, self.getHeaderWidth() + rowHeaderCellWidth, self.visibleRowHeights[aCell.rowIndex]);
}
} else {
self.ctx.lineWidth = self.style.activeCellOverlayBorderWidth;
Expand Down
43 changes: 22 additions & 21 deletions test/tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,15 @@
{col1: 'baz', col2: 2, col3: 'c'}
];
};
// Get color `c` of rgb vector `v`
// Note: See c = {...} above for color options

// Get color `c` of rgb vector `v`
// Note: See c = {...} above for color options
function getC(v) {
return Object.keys(c).filter(function (k) {
return c[k] === v;
})[0] || v;
}

// Convert number `n` to 'spreadsheet-style' column label `s`
// Note: Zero-index, so 0 = A, 27 = AB, etc.
function itoa(n) {
Expand All @@ -55,7 +55,7 @@
}
return s;
}

// Create data grid with `r` rows, `c` columns, and cell contents derived
// by the function `dFn`.
// Note: If dFn does not exist, each cell is left blank.
Expand All @@ -69,7 +69,7 @@
}
return d;
}

// Reset test environment
function cleanup(done) {
var m = document.getElementById('mocha');
Expand All @@ -79,13 +79,13 @@
}
done();
}

// Draws a 'crosshairs' marker at coordinates (x,y).
// The marker includes:
// - A 1px vertical line at x
// - A 1px horizontal line at y
// - A 3px central marker centered at (x,y)
// Note: markerColors[...] selection ensures contrast between lines and
// Note: markerColors[...] selection ensures contrast between lines and
// central marker
function marker(grid, x, y) {
grid.markerCount = grid.markerCount || 0;
Expand Down Expand Up @@ -960,9 +960,9 @@
['ArrowDown', 'Enter'].forEach(function (key) {
var ev = new Event('keydown');
ev.key = key;

i.dispatchEvent(ev);

if (key === 'Enter') {
err = assertIf(grid.data[0].col1 !== 'baz', 'Expected key combination to filter for baz');
}
Expand All @@ -989,7 +989,7 @@
ev.key = key;

i.dispatchEvent(ev);

if (key === 'Enter') {
err = assertIf(grid.data[0].col1 !== 'bar', 'Expected key combination to filter for bar');
}
Expand Down Expand Up @@ -1052,7 +1052,7 @@
it('Autocomplete should have an option for filtering blank values', function (done) {
var grid = g({
test: this.test,
data: [
data: [
{col1: 'bar', col2: 0, col3: 'a'},
{col1: ' ', col2: 1, col3: 'b'},
{col1: 'baz', col2: 2, col3: 'c'},
Expand Down Expand Up @@ -1806,7 +1806,7 @@
test: this.test,
data: smallData()
});

grid.focus();
grid.selectArea({
top: 0, left: 0, bottom: 0, right: 0
Expand All @@ -1815,9 +1815,9 @@
var ev = new Event('keydown');
ev.shiftKey = true;
ev.key = "ArrowRight";

grid.controlInput.dispatchEvent(ev);

done(assertIf(grid.selectedRows.length !== 1 || grid.selections[0].col3 !== undefined, 'Expected the active cell to move.'));
});
it('Shift and Arrow left should add the selection to the left one', function (done) {
Expand Down Expand Up @@ -2696,8 +2696,8 @@
}
});
var cell = grid.getVisibleCellByIndex(0,0)


var lineCount = cell.text.lines.length;
done(
assertIf(lineCount !== 1, 'Expected 1 wrapped lines, got', lineCount)
Expand Down Expand Up @@ -2744,7 +2744,7 @@
});
var cell = grid.getVisibleCellByIndex(0,0)
var lineCount = cell.text.lines.length;

done(
assertIf(lineCount !== 3, 'Expected 3 wrapped lines, got %s', lineCount)
)
Expand Down Expand Up @@ -2773,7 +2773,8 @@
grid.addEventListener('expandtree', function (e) {
assertIf(e.treeGrid === undefined, 'Expected a grid here.');
e.treeGrid.style.cornerCellBackgroundColor = c.y;
assertPxColor(grid, 10, 34, c.fu, function () {

assertPxColor(grid, 10, 38, c.fu, function () {
setTimeout(function () {
assertPxColor(grid, 60, 60, c.y, done);
}, 3);
Expand Down Expand Up @@ -3091,9 +3092,9 @@
mousemove(document.body, 10, 90, grid.canvas);
assertPxColor(grid, 10, 98, c.b, function (err) {
if (err) { return done(err); }
assertPxColor(grid, 10, 85, c.fu, function (err) {
assertPxColor(grid, 10, 86, c.fu, function (err) {
if (err) { return done(err); }
assertPxColor(grid, 30, 90, c.y, done);
assertPxColor(grid, 30, 91, c.y, done);
});
});
grid.draw();
Expand Down

0 comments on commit f70ba1b

Please sign in to comment.