Skip to content

Commit

Permalink
Fix actualBoundingBoxLeft/Right with center/right alignment (#2109)
Browse files Browse the repository at this point in the history
This bug goes back 10 years to the original implementation.

Fixes #1909
  • Loading branch information
zbjornson committed Sep 6, 2022
1 parent 561d933 commit 6862532
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ project adheres to [Semantic Versioning](http://semver.org/).
### Changed
### Added
### Fixed
* Fix `actualBoundingBoxLeft` and `actualBoundingBoxRight` when `textAlign='center'` or `'right'` ([#1909](https://github.com/Automattic/node-canvas/issues/1909))

2.10.0
==================
Expand Down
6 changes: 3 additions & 3 deletions src/CanvasRenderingContext2d.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2748,7 +2748,7 @@ NAN_METHOD(Context2d::MeasureText) {
double x_offset;
switch (context->state->textAlignment) {
case 0: // center
x_offset = logical_rect.width / 2;
x_offset = logical_rect.width / 2.;
break;
case 1: // right
x_offset = logical_rect.width;
Expand All @@ -2766,10 +2766,10 @@ NAN_METHOD(Context2d::MeasureText) {
Nan::New<Number>(logical_rect.width)).Check();
Nan::Set(obj,
Nan::New<String>("actualBoundingBoxLeft").ToLocalChecked(),
Nan::New<Number>(x_offset - PANGO_LBEARING(ink_rect))).Check();
Nan::New<Number>(PANGO_LBEARING(ink_rect) + x_offset)).Check();
Nan::Set(obj,
Nan::New<String>("actualBoundingBoxRight").ToLocalChecked(),
Nan::New<Number>(x_offset + PANGO_RBEARING(ink_rect))).Check();
Nan::New<Number>(PANGO_RBEARING(ink_rect) - x_offset)).Check();
Nan::Set(obj,
Nan::New<String>("actualBoundingBoxAscent").ToLocalChecked(),
Nan::New<Number>(y_offset + PANGO_ASCENT(ink_rect))).Check();
Expand Down
37 changes: 34 additions & 3 deletions test/canvas.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ const {
deregisterAllFonts
} = require('../')

function assertApprox(actual, expected, tol) {
assert(Math.abs(expected - actual) <= tol,
"Expected " + actual + " to be " + expected + " +/- " + tol);
}

describe('Canvas', function () {
// Run with --expose-gc and uncomment this line to help find memory problems:
// afterEach(gc);
Expand Down Expand Up @@ -946,20 +951,46 @@ describe('Canvas', function () {
let metrics = ctx.measureText('Alphabet')
// Actual value depends on font library version. Have observed values
// between 0 and 0.769.
assert.ok(metrics.alphabeticBaseline >= 0 && metrics.alphabeticBaseline <= 1)
assertApprox(metrics.alphabeticBaseline, 0.5, 0.5)
// Positive = going up from the baseline
assert.ok(metrics.actualBoundingBoxAscent > 0)
// Positive = going down from the baseline
assert.ok(metrics.actualBoundingBoxDescent > 0) // ~4-5
assertApprox(metrics.actualBoundingBoxDescent, 5, 2)

ctx.textBaseline = 'bottom'
metrics = ctx.measureText('Alphabet')
assert.strictEqual(ctx.textBaseline, 'bottom')
assert.ok(metrics.alphabeticBaseline > 0) // ~4-5
assertApprox(metrics.alphabeticBaseline, 5, 2)
assert.ok(metrics.actualBoundingBoxAscent > 0)
// On the baseline or slightly above
assert.ok(metrics.actualBoundingBoxDescent <= 0)
})

it('actualBoundingBox is correct for left, center and right alignment (#1909)', function () {
const canvas = createCanvas(0, 0)
const ctx = canvas.getContext('2d')

// positive actualBoundingBoxLeft indicates a distance going left from the
// given alignment point.

// positive actualBoundingBoxRight indicates a distance going right from
// the given alignment point.

ctx.textAlign = 'left'
const lm = ctx.measureText('aaaa')
assertApprox(lm.actualBoundingBoxLeft, -1, 6)
assertApprox(lm.actualBoundingBoxRight, 21, 6)

ctx.textAlign = 'center'
const cm = ctx.measureText('aaaa')
assertApprox(cm.actualBoundingBoxLeft, 9, 6)
assertApprox(cm.actualBoundingBoxRight, 11, 6)

ctx.textAlign = 'right'
const rm = ctx.measureText('aaaa')
assertApprox(rm.actualBoundingBoxLeft, 19, 6)
assertApprox(rm.actualBoundingBoxRight, 1, 6)
})
})

it('Context2d#fillText()', function () {
Expand Down
14 changes: 13 additions & 1 deletion test/public/tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -2658,7 +2658,8 @@ tests['measureText()'] = function (ctx) {
const metrics = ctx.measureText(text)
ctx.strokeStyle = 'blue'
ctx.strokeRect(
x - metrics.actualBoundingBoxLeft + 0.5,
// positive numbers for actualBoundingBoxLeft indicate a distance going left
x + metrics.actualBoundingBoxLeft + 0.5,
y - metrics.actualBoundingBoxAscent + 0.5,
metrics.width,
metrics.actualBoundingBoxAscent + metrics.actualBoundingBoxDescent
Expand All @@ -2677,8 +2678,19 @@ tests['measureText()'] = function (ctx) {
drawWithBBox('Alphabet bottom', 20, 90)

ctx.textBaseline = 'alphabetic'
ctx.save()
ctx.rotate(Math.PI / 8)
drawWithBBox('Alphabet', 50, 100)
ctx.restore()

ctx.textAlign = 'center'
drawWithBBox('Centered', 100, 195)

ctx.textAlign = 'left'
drawWithBBox('Left', 10, 195)

ctx.textAlign = 'right'
drawWithBBox('right', 195, 195)
}

tests['image sampling (#1084)'] = function (ctx, done) {
Expand Down

0 comments on commit 6862532

Please sign in to comment.