fix(images): respect EXIF orientation in thumbnails, heroes and previews (#1194)
* fix(images): respect EXIF orientation in thumbnails, heroes and previews (#1185) generateThumbnail, generateHeroImage and generatePreviewImage went straight from sharp(imagePath) to .resize(), so a photo whose Orientation tag is not 1 — routine for portrait shots on bodies that tag rather than rotate the sensor data — was resized from the raw frame and came out sideways. The same pipelines then call .withMetadata(false), stripping the tag from the output, so nothing downstream could correct it either. The download path already had this right: resizeToBox calls probe.rotate() for stills, which is why the same photo looked correct on download and rotated in the gallery. All three generators now do the same, guarded to stills for the reason resizeToBox already documents — .rotate() flattens a multi-frame source. The reporter also spotted the half that compounds it: photos.width/height were stored from sharp's metadata, which reports pixels as STORED, not as displayed. For orientation 5-8 those are swapped, so a portrait photo landed in the database as landscape and masonry/justified sized its tile with the wrong aspect ratio on top of the image being unrotated. A shared orientedDimensions() helper now does that conversion at all four capture sites — managed upload, background processing, external import and the dimension repair — so the stored numbers describe the rotated result the generators now produce. Existing rows keep their pre-rotation dimensions until the photo is reprocessed; the images themselves correct on the next thumbnail/preview regeneration. Tests fail on the unfixed generators — verified by reverting the rotate calls and the swap, which fails 4 of the 7. * fix(images): orient dimensions on every ingest path, and stop guarding rotate where it protects nothing (#1185) Review found the first cut covered four of eight dimension-capture sites. The filesystem watcher, the S3 auto-importer, the v1 upload API and replace-by-name all still persisted raw metadata.width/height, so an orientation 5-8 photo arriving that way got a correctly rotated thumbnail and a database row describing it as landscape — the same aspect-ratio mismatch this PR set out to remove, just on the paths I had not grepped. (I searched for `metadata.width` and the v1 route aliases it to `meta`.) The animated guard was also wrong in two of the three generators. generateThumbnail and generateHeroImage never pass `animated: true`, so they already flatten a multi-frame source to its first frame — skipping .rotate() there protected an animation that was being discarded anyway, while leaving the output in raw orientation against swapped stored dimensions. Both now rotate unconditionally. generatePreviewImage keeps the guard, because it genuinely does open animated sources as animated and .rotate() would flatten them. That leaves one corner unsolved rather than papered over: a multi-frame source that also carries an orientation tag keeps its raw orientation in the preview while the thumbnail and stored dimensions describe the rotated one. GIF has no EXIF and animated WebP effectively never sets it, so it is a real gap but not a common one, and closing it means rotating frame by frame rather than quietly dropping the animation. Documented at the guard. * fix(images): add a recompute mode so existing libraries get corrected too (#1185) The orientation fix only helped new photos. A row affected by the bug has BOTH dimensions stored — just in the raw order — so the repair job's NULL filter could never reach exactly the rows that needed it. Worse, once their thumbnails regenerated rotated, those rows went from consistently-wrong (sideways image in a matching tile) to inconsistent: correct image, wrong-shaped tile. `recompute` widens the candidate set to every image row. Opt-in, because it re-reads every original. It also has to deal with the consequence for faces. Detection runs against the preview and stores boxes in ORIGINAL pixel space, scaled by `photo.width / previewMeta.width` (faceProcessor.js:220-224) — so a photo whose stored dimensions change has face data recorded against a coordinate system that no longer exists, and the overlays crop the wrong region. Photos whose dimensions actually change are requeued for scanning; ones that were already correct are not, or a routine repair would rescan the whole library. Rows with face_status NULL are left alone so installs that never enabled the feature don't start scanning because of a dimension repair. Writing the test for that last rule caught a real bug in it: the candidate query never selected photos.width/height, so `photo.width` was undefined and every row compared as changed. Both columns are selected now. * Revert "fix(images): add a recompute mode so existing libraries get corrected too (#1185)" This reverts commit cb771d08. Review round 3 found five problems, all of them in this addition rather than in the orientation fix itself, and one of them an own-goal: requeueing face scanning makes processPhotoFaces call ensurePreviewImage, which returns the CACHED pre-fix preview when it is still a valid image — so the rescan reads unrotated pixels and scales those boxes by the newly corrected dimensions. That is worse than leaving the data alone. The rest need work this PR should not be carrying: the dimension repair reads originals through resolvePhotoFilePath and plain sharp, so it does nothing on an S3 install and rejects RAW/DNG; recompute pulls archived rows whose originals were deleted on archive; orientation 2, 3 and 4 change the pixels without changing width or height, so a dimension-delta test never notices them; and the dimension write and the face invalidation are not atomic, so a failure between them leaves a row that no retry will ever requeue. Split out so it can be designed and reviewed on its own. The orientation fix — .rotate() in the three generators and orientedDimensions() at all eight ingest sites — is unaffected and stays. * fix(images): the watermarked rendition needs orienting too (#1185) A fourth generator with the same bug, found while reviewing the backfill that builds on this. watermarkService composites and re-encodes through its own sharp pipeline with no .rotate(), and gallery.js serves photos.watermark_path ahead of the original when branding watermarking is on — so on a watermarked gallery the sideways image is precisely what a guest sees. Two details this needed beyond the .rotate() itself: metadata() is read from a separate, unrotated handle. .rotate() does not change what metadata() reports — a 400x200 source tagged orientation 6 still reads 400x200 — and every use of those numbers here is positioning: watermark scale, font size, composite extent. They have to be the DISPLAYED dimensions or the mark is placed against the wrong axis, so they go through orientedDimensions. The composite offsets are floored. getPositionCoordinates derives from the SVG's estimated text extent and returns fractional pixels; sharp rejects a non-integer offset and applyWatermark catches its own error and returns the image unwatermarked. Landing on a whole pixel was luck, and changing the dimensions it is computed from ran out of it — the test surfaced a real "Expected integer for left but received 92.8". --------- Co-authored-by: Paul Nothaft <[email protected]>
This commit is contained in:
Paul Nothaft
co-authored by
Paul Nothaft
Paul Nothaft
parent
2d98f5fa9f
commit
c18f54ede0
@@ -0,0 +1,159 @@
|
||||
/**
|
||||
* EXIF orientation in the gallery-facing generators (#1185).
|
||||
*
|
||||
* sharp decodes the pixels as stored, not as displayed. A photo whose
|
||||
* Orientation tag is not 1 — routine for portrait shots on bodies that tag
|
||||
* rather than rotate the sensor data — therefore resizes from the raw frame
|
||||
* and comes out sideways. The generators then call `.withMetadata(false)`,
|
||||
* which strips the tag from the output, so the browser has no hint left to
|
||||
* correct it either: nothing downstream can recover it.
|
||||
*
|
||||
* The download path (`resizeToBox`) always got this right. These three did
|
||||
* not, which is why the same photo looked correct on download and rotated in
|
||||
* the gallery.
|
||||
*
|
||||
* Every assertion here fails on the unfixed generators.
|
||||
*/
|
||||
|
||||
const fs = require('fs').promises;
|
||||
const path = require('path');
|
||||
const os = require('os');
|
||||
const sharp = require('sharp');
|
||||
|
||||
// imageProcessor reads its thumbnail settings from app_settings, so without a
|
||||
// db the require() alone hangs the run. Same shape the other generator tests
|
||||
// use (ensureHeroImage.external.test.js).
|
||||
jest.mock('../../src/database/db', () => {
|
||||
const api = (table) => {
|
||||
if (table === 'app_settings') {
|
||||
return { where: () => ({ whereIn: () => [], first: async () => null }), whereIn: async () => [] };
|
||||
}
|
||||
if (table === 'events') return { where: () => ({ first: async () => null }) };
|
||||
if (table === 'photos') return { where: () => ({ update: async () => 1, first: async () => null }) };
|
||||
return { where: () => ({ first: async () => null }) };
|
||||
};
|
||||
return { db: api };
|
||||
});
|
||||
|
||||
const LocalFsStorage = require('../../src/services/storage/LocalFsStorage');
|
||||
const storageModule = require('../../src/services/storage');
|
||||
|
||||
describe('EXIF orientation in thumbnails, heroes and previews (#1185)', () => {
|
||||
let tmpDir;
|
||||
let imageProcessor;
|
||||
let landscapeTaggedPortrait;
|
||||
|
||||
// 400x200 as stored, Orientation 6 (90° CW) — so it DISPLAYS as 200x400.
|
||||
// This is exactly the shape the reporter's Sony bodies produce: the sensor
|
||||
// data is landscape and the tag carries the rotation.
|
||||
const W = 400;
|
||||
const H = 200;
|
||||
|
||||
let storageRoot;
|
||||
|
||||
beforeAll(async () => {
|
||||
tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'picpeak-orient-'));
|
||||
storageRoot = path.join(tmpDir, 'storage');
|
||||
await fs.mkdir(storageRoot, { recursive: true });
|
||||
process.env.STORAGE_PATH = storageRoot;
|
||||
|
||||
const storage = new LocalFsStorage({ root: storageRoot });
|
||||
await storage.init();
|
||||
storageModule.setStorageForTesting(storage);
|
||||
|
||||
landscapeTaggedPortrait = path.join(tmpDir, 'portrait-tagged.jpg');
|
||||
await sharp({
|
||||
create: { width: W, height: H, channels: 3, background: { r: 120, g: 80, b: 40 } },
|
||||
})
|
||||
.withMetadata({ orientation: 6 })
|
||||
.jpeg()
|
||||
.toFile(landscapeTaggedPortrait);
|
||||
|
||||
delete require.cache[require.resolve('../../src/services/imageProcessor')];
|
||||
imageProcessor = require('../../src/services/imageProcessor');
|
||||
}, 60000);
|
||||
|
||||
afterAll(async () => {
|
||||
await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => {});
|
||||
});
|
||||
|
||||
test('the fixture really is stored landscape with a rotation tag', async () => {
|
||||
const m = await sharp(landscapeTaggedPortrait).metadata();
|
||||
expect(m.width).toBe(W);
|
||||
expect(m.height).toBe(H);
|
||||
expect(m.orientation).toBe(6);
|
||||
});
|
||||
|
||||
test('orientedDimensions reports what a viewer sees, not what is stored', async () => {
|
||||
const m = await sharp(landscapeTaggedPortrait).metadata();
|
||||
// Swapped: this is what the grid needs to size a tile, and what the
|
||||
// database should hold.
|
||||
expect(imageProcessor.orientedDimensions(m)).toEqual({ width: H, height: W });
|
||||
});
|
||||
|
||||
test('orientedDimensions leaves an untagged image alone', async () => {
|
||||
const plain = path.join(tmpDir, 'plain.jpg');
|
||||
await sharp({ create: { width: W, height: H, channels: 3, background: { r: 1, g: 2, b: 3 } } })
|
||||
.jpeg().toFile(plain);
|
||||
const m = await sharp(plain).metadata();
|
||||
expect(imageProcessor.orientedDimensions(m)).toEqual({ width: W, height: H });
|
||||
});
|
||||
|
||||
test('orientedDimensions survives metadata it cannot use', () => {
|
||||
expect(imageProcessor.orientedDimensions(null)).toEqual({ width: null, height: null });
|
||||
expect(imageProcessor.orientedDimensions({})).toEqual({ width: null, height: null });
|
||||
});
|
||||
|
||||
test('the thumbnail comes out portrait, not sideways', async () => {
|
||||
const rel = await imageProcessor.generateThumbnail(landscapeTaggedPortrait, {
|
||||
outputBasename: 'orient-thumb.jpg',
|
||||
regenerate: true,
|
||||
});
|
||||
expect(rel).toBeTruthy();
|
||||
|
||||
const out = await sharp(path.join(storageRoot, rel)).metadata();
|
||||
// Unfixed, this came back wider than tall — the raw frame, resized.
|
||||
expect(out.height).toBeGreaterThan(out.width);
|
||||
});
|
||||
|
||||
test('the preview tier comes out portrait too', async () => {
|
||||
const rel = await imageProcessor.generatePreviewImage(landscapeTaggedPortrait, {
|
||||
outputBasename: 'orient-preview.jpg',
|
||||
});
|
||||
expect(rel).toBeTruthy();
|
||||
|
||||
const out = await sharp(path.join(storageRoot, rel)).metadata();
|
||||
expect(out.height).toBeGreaterThan(out.width);
|
||||
});
|
||||
|
||||
test('the watermarked rendition is oriented too', async () => {
|
||||
// gallery.js serves photos.watermark_path ahead of the original when
|
||||
// branding watermarking is on, so this is the rendition a guest actually
|
||||
// sees — and it went through its own sharp pipeline that nobody had
|
||||
// rotated.
|
||||
const watermarkService = require('../../src/services/watermarkService');
|
||||
const buf = await watermarkService.applyWatermark(landscapeTaggedPortrait, {
|
||||
enabled: true, position: 'bottom-right', opacity: 50, size: 15,
|
||||
// companyName, not text — the SVG branch reads this one, and without it
|
||||
// the service falls through without compositing anything.
|
||||
companyName: 'PicPeak',
|
||||
});
|
||||
expect(Buffer.isBuffer(buf)).toBe(true);
|
||||
|
||||
const out = await sharp(buf).metadata();
|
||||
// 400x200 stored, tagged 6 — so the watermarked output must be portrait.
|
||||
expect(out.height).toBeGreaterThan(out.width);
|
||||
});
|
||||
|
||||
test('the orientation tag is gone from the output, so nothing double-rotates', async () => {
|
||||
// The pixels are corrected now, so a surviving tag would make a viewer
|
||||
// rotate an already-rotated image. withMetadata(false) strips it; this
|
||||
// pins that the two changes agree.
|
||||
const rel = await imageProcessor.generateThumbnail(landscapeTaggedPortrait, {
|
||||
outputBasename: 'orient-thumb-tag.jpg',
|
||||
regenerate: true,
|
||||
});
|
||||
const out = await sharp(path.join(storageRoot, rel)).metadata();
|
||||
expect(out.orientation === undefined || out.orientation === 1).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -73,6 +73,15 @@ jest.mock('../../src/services/imageProcessor', () => {
|
||||
generateThumbnail: mockGenerateThumbnail,
|
||||
generateVideoPlaceholder: jest.fn(async (filename) => `thumbnails/thumb_${filename.replace(/\.[^.]+$/, '')}.jpg`),
|
||||
extractCaptureDate: mockExtractCaptureDate,
|
||||
// processPhoto routes stored dimensions through this to get the displayed
|
||||
// ones (#1185). Mirrored rather than requireActual'd, because pulling the
|
||||
// real module in here would drag its database dependency into the mock
|
||||
// factory. Kept faithful to imageProcessor.orientedDimensions.
|
||||
orientedDimensions: jest.fn((m) => {
|
||||
if (!m || !m.width || !m.height) return { width: null, height: null };
|
||||
const swap = m.orientation >= 5 && m.orientation <= 8;
|
||||
return { width: swap ? m.height : m.width, height: swap ? m.width : m.height };
|
||||
}),
|
||||
withLocalCopy: jest.fn(async (key, fn) =>
|
||||
fn(`/tmp/local-copy-${require('path').basename(key)}`)
|
||||
),
|
||||
|
||||
@@ -8,7 +8,7 @@ const { list, resolveExternalPath, getExternalMediaRoot } = require('../services
|
||||
const { db, logActivity } = require('../database/db');
|
||||
const sharp = require('sharp');
|
||||
const logger = require('../utils/logger');
|
||||
const { generateThumbnail, extractCaptureDate } = require('../services/imageProcessor');
|
||||
const { generateThumbnail, extractCaptureDate, orientedDimensions } = require('../services/imageProcessor');
|
||||
const { isUniqueViolation } = require('../utils/dbErrors');
|
||||
|
||||
const router = express.Router();
|
||||
@@ -202,8 +202,10 @@ router.post('/events/:id/import-external', adminAuth, requirePermission('photos.
|
||||
let height = null;
|
||||
try {
|
||||
const metadata = await sharp(f.full).metadata();
|
||||
width = metadata.width || null;
|
||||
height = metadata.height || null;
|
||||
// Oriented, not raw: a portrait shot from a body that tags rather
|
||||
// than rotates reports landscape dimensions, and the grid would size
|
||||
// its tile from those (#1185).
|
||||
({ width, height } = orientedDimensions(metadata));
|
||||
} catch (dimErr) {
|
||||
logger.warn(`Could not extract dimensions for ${f.rel}: ${dimErr.message}`);
|
||||
}
|
||||
|
||||
@@ -170,13 +170,15 @@ router.post('/repair-dimensions', adminAuth, requirePermission('system.manage'),
|
||||
}
|
||||
|
||||
const metadata = await sharp(fullPath).metadata();
|
||||
// Oriented, not raw — see orientedDimensions (#1185).
|
||||
const dims = require('../services/imageProcessor').orientedDimensions(metadata);
|
||||
|
||||
if (metadata.width && metadata.height) {
|
||||
if (dims.width && dims.height) {
|
||||
await db('photos')
|
||||
.where({ id: photo.id })
|
||||
.update({
|
||||
width: metadata.width,
|
||||
height: metadata.height
|
||||
width: dims.width,
|
||||
height: dims.height
|
||||
});
|
||||
successCount++;
|
||||
|
||||
|
||||
@@ -665,8 +665,8 @@ router.post(
|
||||
let height = null;
|
||||
try {
|
||||
const meta = await sharp(tempPath).metadata();
|
||||
width = meta.width || null;
|
||||
height = meta.height || null;
|
||||
// Oriented, not raw — see imageProcessor.orientedDimensions (#1185).
|
||||
({ width, height } = require('../../services/imageProcessor').orientedDimensions(meta));
|
||||
} catch { /* non-fatal */ }
|
||||
|
||||
let thumbRel = null;
|
||||
|
||||
@@ -112,8 +112,10 @@ async function processNewPhoto(filePath) {
|
||||
if (!isVideo) {
|
||||
try {
|
||||
const metadata = await sharp(filePath).metadata();
|
||||
if (metadata.width && metadata.height) {
|
||||
dimensions = { width: metadata.width, height: metadata.height };
|
||||
// Oriented, not raw — see imageProcessor.orientedDimensions (#1185).
|
||||
const dims = require('./imageProcessor').orientedDimensions(metadata);
|
||||
if (dims.width && dims.height) {
|
||||
dimensions = { width: dims.width, height: dims.height };
|
||||
}
|
||||
} catch (err) {
|
||||
logger.debug(`Could not read image dimensions for ${filename}: ${err.message}`);
|
||||
|
||||
@@ -260,6 +260,22 @@ async function generateThumbnail(imagePath, options = {}) {
|
||||
failOn: 'none'
|
||||
});
|
||||
|
||||
// Apply EXIF orientation before resizing (#1185). Without this a photo
|
||||
// whose Orientation tag is not 1 — routine for portrait shots on bodies
|
||||
// that tag rather than rotate the sensor data — is resized from the raw
|
||||
// pixels and comes out sideways. `.withMetadata(false)` below then strips
|
||||
// the tag, so the browser has no hint left to correct it either, which is
|
||||
// why this cannot be left to the client.
|
||||
//
|
||||
// Unconditional, unlike resizeToBox and generatePreviewImage. Those open
|
||||
// multi-frame sources with `animated: true` and must not rotate them,
|
||||
// because `.rotate()` flattens to the first frame. This one never passes
|
||||
// that option, so it already produces a still — guarding on `pages` here
|
||||
// would protect an animation that was being discarded anyway, and leave
|
||||
// the thumbnail in raw orientation while the stored dimensions describe
|
||||
// the rotated one.
|
||||
sharpInstance = sharpInstance.rotate();
|
||||
|
||||
// Strip EXIF/metadata from thumbnails (privacy: prevent GPS leak etc.)
|
||||
sharpInstance = sharpInstance.withMetadata(false);
|
||||
|
||||
@@ -338,6 +354,31 @@ async function isThumbnailValid(thumbnailPath) {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The dimensions a viewer actually sees, given EXIF orientation (#1185).
|
||||
*
|
||||
* sharp reports `metadata.width`/`height` as the pixels are stored, not as
|
||||
* they are displayed. Orientation values 5-8 carry a 90° rotation, so for
|
||||
* those the two are swapped — which is why a portrait photo from a body that
|
||||
* tags rather than rotates was landing in the database as landscape, and why
|
||||
* masonry and justified layouts sized its tile with the wrong aspect ratio on
|
||||
* top of the image itself being unrotated.
|
||||
*
|
||||
* Everything that renders these photos now applies `.rotate()`, so the stored
|
||||
* numbers have to describe the rotated result to match.
|
||||
*
|
||||
* @param {Object} metadata - a sharp metadata object
|
||||
* @returns {{ width: number|null, height: number|null }}
|
||||
*/
|
||||
function orientedDimensions(metadata) {
|
||||
if (!metadata || !metadata.width || !metadata.height) return { width: null, height: null };
|
||||
const swap = metadata.orientation >= 5 && metadata.orientation <= 8;
|
||||
return {
|
||||
width: swap ? metadata.height : metadata.width,
|
||||
height: swap ? metadata.width : metadata.height,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Wraps a callback that needs the source image as a local file. In local-fs
|
||||
* mode the storage path is used directly (no copy); in S3 mode the object is
|
||||
@@ -514,6 +555,11 @@ async function generateHeroImage(imagePath, options = {}) {
|
||||
failOn: 'none'
|
||||
});
|
||||
|
||||
// EXIF orientation, same reasoning as generateThumbnail (#1185) — and
|
||||
// unconditional for the same reason: no `animated: true` on the input, so
|
||||
// this output is a still whatever the source was.
|
||||
sharpInstance = sharpInstance.rotate();
|
||||
|
||||
// Strip EXIF/metadata from hero images (privacy: prevent GPS leak etc.)
|
||||
sharpInstance = sharpInstance.withMetadata(false);
|
||||
|
||||
@@ -735,6 +781,21 @@ async function generatePreviewImage(imagePath, options = {}) {
|
||||
animated: isAnimated,
|
||||
});
|
||||
|
||||
// EXIF orientation (#1185). Guarded here and not in the thumbnail/hero
|
||||
// generators because this one DOES open multi-frame sources with
|
||||
// `animated: true` above, and `.rotate()` would flatten them to a single
|
||||
// frame — trading an animation for an orientation.
|
||||
//
|
||||
// Which leaves one corner unsolved: a multi-frame source that also carries
|
||||
// an orientation tag keeps its raw orientation here while the thumbnail
|
||||
// and the stored dimensions describe the rotated one. GIF has no EXIF at
|
||||
// all and animated WebP effectively never sets it, so this is a real gap
|
||||
// rather than a common one, and closing it properly means rotating frame
|
||||
// by frame rather than dropping the animation.
|
||||
if (!isAnimated) {
|
||||
sharpInstance = sharpInstance.rotate();
|
||||
}
|
||||
|
||||
// Strip EXIF — same privacy reasoning as thumbnails/heroes.
|
||||
sharpInstance = sharpInstance.withMetadata(false);
|
||||
|
||||
@@ -1243,6 +1304,7 @@ async function resizeToBox(inputBuffer, box, options = {}) {
|
||||
}
|
||||
|
||||
module.exports = {
|
||||
orientedDimensions,
|
||||
ensurePreviewImageAtWidth,
|
||||
ensureThumbnailAtWidth,
|
||||
thumbnailTierKeys,
|
||||
|
||||
@@ -174,11 +174,10 @@ async function processUploadedPhotos(files, eventId, uploadedBy = 'admin', categ
|
||||
try {
|
||||
const sharp = require('sharp');
|
||||
const metadata = await sharp(proc.path).metadata();
|
||||
if (metadata.width && metadata.height) {
|
||||
imageMetadata = {
|
||||
width: metadata.width,
|
||||
height: metadata.height
|
||||
};
|
||||
// Oriented, not raw — see orientedDimensions (#1185).
|
||||
const dims = require('./imageProcessor').orientedDimensions(metadata);
|
||||
if (dims.width && dims.height) {
|
||||
imageMetadata = { width: dims.width, height: dims.height };
|
||||
}
|
||||
} catch (metadataError) {
|
||||
logger.warn(`Could not extract image dimensions for ${file.originalname}:`, metadataError.message);
|
||||
@@ -544,9 +543,11 @@ async function processPhoto(photoId) {
|
||||
try {
|
||||
const sharp = require('sharp');
|
||||
const metadata = await sharp(proc.path).metadata();
|
||||
if (metadata.width && metadata.height) {
|
||||
updateData.width = metadata.width;
|
||||
updateData.height = metadata.height;
|
||||
// Oriented, not raw — see orientedDimensions (#1185).
|
||||
const dims = require('./imageProcessor').orientedDimensions(metadata);
|
||||
if (dims.width && dims.height) {
|
||||
updateData.width = dims.width;
|
||||
updateData.height = dims.height;
|
||||
}
|
||||
} catch (e) {
|
||||
logger.warn(`processPhoto: dimensions extraction failed for ${photoId}`, { error: e.message });
|
||||
|
||||
@@ -78,8 +78,8 @@ async function replacePhoto(existingPhoto, newFileTempPath, { originalFilename,
|
||||
try {
|
||||
try {
|
||||
const metadata = await sharp(proc.path).metadata();
|
||||
width = metadata.width || null;
|
||||
height = metadata.height || null;
|
||||
// Oriented, not raw — see imageProcessor.orientedDimensions (#1185).
|
||||
({ width, height } = require('./imageProcessor').orientedDimensions(metadata));
|
||||
} catch {
|
||||
// Non-image or corrupt
|
||||
}
|
||||
|
||||
@@ -110,8 +110,10 @@ async function processEvent(event, storage) {
|
||||
try {
|
||||
dimensions = await withLocalCopy(entry.key, async (localPath) => {
|
||||
const metadata = await sharp(localPath).metadata();
|
||||
if (metadata.width && metadata.height) {
|
||||
return { width: metadata.width, height: metadata.height };
|
||||
// Oriented, not raw — see imageProcessor.orientedDimensions (#1185).
|
||||
const dims = require('./imageProcessor').orientedDimensions(metadata);
|
||||
if (dims.width && dims.height) {
|
||||
return { width: dims.width, height: dims.height };
|
||||
}
|
||||
return null;
|
||||
});
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
const sharp = require('sharp');
|
||||
const { orientedDimensions } = require('./imageProcessor');
|
||||
const path = require('path');
|
||||
const fs = require('fs').promises;
|
||||
const { db } = require('../database/db');
|
||||
@@ -115,9 +116,24 @@ class WatermarkService {
|
||||
}
|
||||
}
|
||||
|
||||
// Load the main image
|
||||
const image = sharp(imagePath);
|
||||
const metadata = await image.metadata();
|
||||
// Load the main image.
|
||||
//
|
||||
// .rotate() for the same reason as the other generators (#1185): sharp
|
||||
// decodes the pixels as stored, so an orientation-tagged photo would be
|
||||
// composited and re-encoded sideways — and gallery.js serves
|
||||
// watermark_path ahead of the original, so this is exactly what a guest
|
||||
// sees.
|
||||
const image = sharp(imagePath).rotate();
|
||||
|
||||
// Deliberately NOT `await image.metadata()`: .rotate() does not change
|
||||
// what metadata() reports — a 400x200 source tagged orientation 6 still
|
||||
// reads 400x200 there, even though the pipeline now emits 200x400. Every
|
||||
// use below is positioning (watermark scale, font size, composite
|
||||
// extent), so it has to be the DISPLAYED size or the mark lands against
|
||||
// the wrong axis.
|
||||
const rawMetadata = await sharp(imagePath).metadata();
|
||||
const oriented = orientedDimensions(rawMetadata);
|
||||
const metadata = { ...rawMetadata, width: oriented.width, height: oriented.height };
|
||||
|
||||
let watermarkBuffer;
|
||||
let watermarkMetadata;
|
||||
@@ -190,11 +206,17 @@ class WatermarkService {
|
||||
settings.position
|
||||
);
|
||||
|
||||
// Apply watermark with high quality output to preserve original image quality
|
||||
// Apply watermark with high quality output to preserve original image quality.
|
||||
//
|
||||
// Floored: getPositionCoordinates derives from the SVG's estimated text
|
||||
// extent, which is fractional, and sharp rejects a non-integer offset
|
||||
// outright — applyWatermark then catches its own error and silently
|
||||
// returns the unwatermarked original. Whether it lands on a whole pixel
|
||||
// was previously luck; nothing guaranteed it.
|
||||
let watermarkedImage = image.composite([{
|
||||
input: watermarkBuffer,
|
||||
top: position.top,
|
||||
left: position.left
|
||||
top: Math.max(0, Math.floor(position.top)),
|
||||
left: Math.max(0, Math.floor(position.left))
|
||||
}]);
|
||||
|
||||
// Preserve original format with high quality settings
|
||||
|
||||
Reference in New Issue
Block a user