From 2163ae2662d1cc94efb4c9176b05cb9502d8e530 Mon Sep 17 00:00:00 2001 From: Ken Peng <37787058+pyk1998@users.noreply.github.com> Date: Mon, 15 Apr 2024 18:44:32 +0800 Subject: [PATCH] Fix forward SyncTex accuracy in internal browser by providing a range location indication alternative (#4194) * Add forward SyncTex with range location indication. * Remain default behavior of using red circle for forward SyncTex. * Remove unused column parameter in callSyncTeXToPDFRange. * Merged configuration `latex-workshop.synctex.indicator.type` to `latex-workshop.synctex.indicator.enabled`. Added fallback logic in `toPDF` for boolean type of `latex-workshop.synctex.indicator.enabled`. Extend type `SyncTeXRecordToPDFAll` from base type `SyncTeXRecordToPDF`. Merged `locateRange` to `locate`. Remove `SyncTeXRecordToPDFAllList`. Merged `callSyncTeXToPDFRange` to `callSyncTeXToPDF`. In `ServerResponse`, added and extended type `SynctexData` from base type `SynctexRangeData`. * Merged `forwardSynctexRange` with `forwardSynctex` in viewer. * Indent fixing. * Change the enum name. Simplify if-else clauses. Change rectangle default color. Add comments. * Code style tweaks --------- Co-authored-by: James Yu --- package.json | 16 ++- src/locate/synctex.ts | 131 +++++++++++++++--- src/preview/viewer.ts | 7 +- src/types.ts | 7 + .../latex-workshop-protocol-types/index.d.ts | 22 ++- viewer/latexworkshop.css | 20 +++ viewer/latexworkshop.ts | 100 +++++++++---- 7 files changed, 243 insertions(+), 60 deletions(-) diff --git a/package.json b/package.json index 0289e616c..ad4b28df5 100644 --- a/package.json +++ b/package.json @@ -1682,9 +1682,19 @@ }, "latex-workshop.synctex.indicator.enabled": { "scope": "window", - "type": "boolean", - "default": true, - "markdownDescription": "Define the visibility of SyncTeX indicator (a red highlighting circle) after a forward SyncTeX in the PDF viewer." + "type": "string", + "enum": [ + "none", + "circle", + "rectangle" + ], + "enumDescriptions": [ + "Hide the indicator.", + "Indicates a possible location with a red circular SyncTeX indicator.", + "Indicates the whole line selected in the Tex file with a red rectangular SyncTeX indicator. (Valid when not using synctex.js)" + ], + "default": "circle", + "markdownDescription": "Define the visibility and style of SyncTeX indicator after a forward SyncTeX in the PDF viewer." }, "latex-workshop.synctex.afterBuild.enabled": { "scope": "window", diff --git a/src/locate/synctex.ts b/src/locate/synctex.ts index bcb072b1b..02645c76a 100644 --- a/src/locate/synctex.ts +++ b/src/locate/synctex.ts @@ -3,7 +3,7 @@ import * as fs from 'fs' import * as path from 'path' import * as cs from 'cross-spawn' import { lw } from '../lw' -import type { SyncTeXRecordToPDF, SyncTeXRecordToTeX } from '../types' +import type { SyncTeXRecordToPDF, SyncTeXRecordToPDFAll, SyncTeXRecordToTeX } from '../types' import { syncTeXToPDF, syncTeXToTeX } from './synctex/worker' import { replaceArgumentPlaceholders } from '../utils/utils' import { isSameRealPath } from '../utils/pathnormalize' @@ -61,6 +61,72 @@ function parseToPDF(result: string): SyncTeXRecordToPDF { } } +/** + * Parse the result of SyncTeX forward to PDF with a list. + * + * This function takes the result of SyncTeX forward to PDF as a string and + * parses it to extract page number, x-coordinate, y-coordinate, box-based + * coordinates (h, v, H, W), and whether the red indicator should be shown in + * the viewer. + * + * @param result - The result string of SyncTeX forward to PDF. + * @returns A SyncTeXRecordToPDFAllList object containing a list of records, + * with each record containing page number, x-coordinate, y-coordinate, + * h-coordinate, v-coordinate, H-coordinate, W-coordinate, and an indicator. + * @throws Error if there is a parsing error. + */ +function parseToPDFList(result: string): SyncTeXRecordToPDFAll[] { + const records: SyncTeXRecordToPDFAll[] = [] + let started = false + let recordIndex = -1 + + for (const line of result.split('\n')) { + if (line.includes('SyncTeX result begin')) { + started = true + continue + } + + if (line.includes('SyncTeX result end')) { + break + } + + if (!started) { + continue + } + + const pos = line.indexOf(':') + if (pos < 0) { + continue + } + + const key = line.substring(0, pos) + const value = line.substring(pos + 1).trim() + + if (key === 'Output') { + recordIndex += 1 + const record: SyncTeXRecordToPDFAll = { page: 0, x: 0, y: 0, h: 0, v: 0, W: 0, H: 0, indicator: true } + records[recordIndex] = record + } + + if (key === 'Page' || key === 'h' || key === 'v' || key === 'W' || key === 'H' || key === 'x' || key === 'y') { + const record = records[recordIndex] + if (record) { + if (key === 'Page') { + record['page'] = Number(value) + } else { + record[key] = Number(value) + } + } + } + } + + if (recordIndex !== -1) { + return records + } else { + throw(new Error('parse error when parsing the result of synctex forward.')) + } +} + /** * Parse the result of SyncTeX backward to TeX. * @@ -169,23 +235,38 @@ function toPDF(args?: {line: number, filePath: string}, forcedViewer: 'auto' | ' const useSyncTexJs = configuration.get('synctex.synctexjs.enabled') as boolean - if (useSyncTexJs) { - try { - logger.log(`Forward from ${filePath} to ${pdfFile} on line ${line}.`) - const record = syncTeXToPDF(line, filePath, pdfFile) - if (!record) { - return - } - void lw.viewer.locate(pdfFile, record) - } catch (e) { - logger.logError('Forward SyncTeX failed.', e) - } + let indicatorType: string + + const indicatorConfig = configuration.get('synctex.indicator.enabled') + + if (typeof indicatorConfig === 'boolean') { + // if configuration is boolean in previous version, then use fallback logic. + indicatorType = indicatorConfig ? 'circle' : 'none' } else { - void callSyncTeXToPDF(line, character, filePath, pdfFile).then( (record) => { - if (pdfFile) { + // if configuration is enum, then use directly. + indicatorType = indicatorConfig as string + } + + // guard if indicatorConfig is illegal or equals to 'none', display none. + if (indicatorType === 'circle' || indicatorType === 'rectangle') { + if (useSyncTexJs) { + try { + logger.log(`Forward from ${filePath} to ${pdfFile} on line ${line}.`) + const record = syncTeXToPDF(line, filePath, pdfFile) + if (!record) { + return + } void lw.viewer.locate(pdfFile, record) + } catch (e) { + logger.logError('Forward SyncTeX failed.', e) } - }) + } else { + void callSyncTeXToPDF(line, character, filePath, pdfFile, indicatorType).then( (record) => { + if (pdfFile) { + void lw.viewer.locate(pdfFile, record) + } + }) + } } } @@ -200,12 +281,20 @@ function toPDF(args?: {line: number, filePath: string}, forcedViewer: 'auto' | ' * @param col - The character position (column) in the line. * @param filePath - The path of the TeX file. * @param pdfFile - The path of the PDF file. - * @returns A promise resolving to a SyncTeXRecordToPDF object. + * @param indicatorType - The type of the SyncTex indicator. + * @returns A promise resolving to a SyncTeXRecordToPDF object or a SyncTeXRecordToPDF[] object. */ -function callSyncTeXToPDF(line: number, col: number, filePath: string, pdfFile: string): Thenable { +function callSyncTeXToPDF(line: number, col: number, filePath: string, pdfFile: string, indicatorType: string): Thenable +function callSyncTeXToPDF(line: number, col: number, filePath: string, pdfFile: string, indicatorType: string): Thenable +function callSyncTeXToPDF(line: number, col: number, filePath: string, pdfFile: string, indicatorType: string): Thenable | Thenable { const configuration = vscode.workspace.getConfiguration('latex-workshop') const docker = configuration.get('docker.enabled') - const args = ['view', '-i', `${line}:${col + 1}:${docker ? path.basename(filePath): filePath}`, '-o', docker ? path.basename(pdfFile): pdfFile] + + const args = ['view', '-i'].concat([ + `${line}${indicatorType === 'rectangle' ? ':0' : `:${col + 1}`}:${docker ? path.basename(filePath) : filePath}`, + '-o', + docker ? path.basename(pdfFile) : pdfFile + ]) let command = configuration.get('synctex.path') as string if (docker) { @@ -236,15 +325,15 @@ function callSyncTeXToPDF(line: number, col: number, filePath: string, pdfFile: logger.logError(`(${logTag}) Forward SyncTeX failed.`, err, stderr) }) - return new Promise( (resolve) => { + return new Promise( (resolve) => { proc.on('exit', exitCode => { if (exitCode !== 0) { logger.logError(`(${logTag}) Forward SyncTeX failed.`, exitCode, stderr) } else { - resolve(parseToPDF(stdout)) + resolve(indicatorType === 'rectangle' ? parseToPDFList(stdout) : parseToPDF(stdout)) } }) - }) + }) as Thenable | Thenable } /** diff --git a/src/preview/viewer.ts b/src/preview/viewer.ts index 6d476747a..7ebab2f91 100644 --- a/src/preview/viewer.ts +++ b/src/preview/viewer.ts @@ -4,7 +4,7 @@ import * as path from 'path' import * as os from 'os' import * as cs from 'cross-spawn' import { lw } from '../lw' -import type { SyncTeXRecordToPDF, ViewerMode } from '../types' +import type { SyncTeXRecordToPDF, SyncTeXRecordToPDFAll, ViewerMode } from '../types' import * as manager from './viewer/pdfviewermanager' import { populate } from './viewer/pdfviewerpanel' @@ -392,7 +392,7 @@ function getParams(): PdfViewerParams { * @param pdfFile The path of a PDF file. * @param record The position to be revealed. */ -async function locate(pdfFile: string, record: SyncTeXRecordToPDF): Promise { +async function locate(pdfFile: string, record: SyncTeXRecordToPDF | SyncTeXRecordToPDFAll[]): Promise { const pdfUri = vscode.Uri.file(pdfFile) let clientSet = manager.getClients(pdfUri) if (clientSet === undefined || clientSet.size === 0) { @@ -407,8 +407,7 @@ async function locate(pdfFile: string, record: SyncTeXRecordToPDF): Promise { - const indicator = vscode.workspace.getConfiguration('latex-workshop').get('synctex.indicator.enabled') as boolean - client.send({type: 'synctex', data: {...record, indicator}}) + client.send({type: 'synctex', data: record}) }, needDelay ? 200 : 0) logger.log(`Try to synctex ${pdfFile}`) } diff --git a/src/types.ts b/src/types.ts index d99fa5a1f..21603b076 100644 --- a/src/types.ts +++ b/src/types.ts @@ -110,6 +110,13 @@ export type SyncTeXRecordToTeX = { column: number } +export type SyncTeXRecordToPDFAll = SyncTeXRecordToPDF & { + h: number, + v: number, + W: number, + H: number +} + export interface LaTeXLinter { readonly linterDiagnostics: vscode.DiagnosticCollection, getName(): string, diff --git a/types/latex-workshop-protocol-types/index.d.ts b/types/latex-workshop-protocol-types/index.d.ts index a127d3996..8d29e59d7 100644 --- a/types/latex-workshop-protocol-types/index.d.ts +++ b/types/latex-workshop-protocol-types/index.d.ts @@ -1,13 +1,23 @@ + +type SynctexData = { + page: number; + x: number; + y: number; + indicator: boolean; +} + +type SynctexRangeData = SynctexData & { + h: number; + v: number; + W: number; + H: number; +} + export type ServerResponse = { type: 'refresh' } | { type: 'synctex', - data: { - page: number, - x: number, - y: number, - indicator: boolean - } + data: SynctexData | SynctexRangeData[] } | { type: 'reload' } diff --git a/viewer/latexworkshop.css b/viewer/latexworkshop.css index 9d98a68c0..dd6e90b07 100644 --- a/viewer/latexworkshop.css +++ b/viewer/latexworkshop.css @@ -54,6 +54,26 @@ html[dir='rtl'] .findbar { transform: translate(-50%, -50%); } +@keyframes synctex-indicator-fadeOut { + 0% { + background-color: rgba(255, 0, 0, 0.4); + } + 25% { + background-color: rgba(255, 0, 0, 0.4); + } + 100% { + background-color: rgba(0, 0, 0, 0); + } +} + +.synctex-indicator-rect { + position: absolute; + z-index: 100000; + background-color: rgba(0, 0, 255, 0.5); + pointer-events: none; + animation: synctex-indicator-fadeOut 1s forwards; +} + #synctex-indicator.show { transition: none; opacity: 0.8; diff --git a/viewer/latexworkshop.ts b/viewer/latexworkshop.ts index 2df9202ee..d07cc6999 100644 --- a/viewer/latexworkshop.ts +++ b/viewer/latexworkshop.ts @@ -7,7 +7,7 @@ import {ExternalPromise} from './components/externalpromise.js' import {ViewerHistory} from './components/viewerhistory.js' import type {PdfjsEventName, IDisposable, ILatexWorkshopPdfViewer, IPDFViewerApplication, IPDFViewerApplicationOptions} from './components/interface.js' -import type {ClientRequest, ServerResponse, PanelManagerResponse, PanelRequest, PdfViewerParams, PdfViewerState} from '../types/latex-workshop-protocol-types/index' +import type {ClientRequest, ServerResponse, PanelManagerResponse, PanelRequest, PdfViewerParams, PdfViewerState, SynctexData, SynctexRangeData} from '../types/latex-workshop-protocol-types/index' declare const PDFViewerApplication: IPDFViewerApplication declare const PDFViewerApplicationOptions: IPDFViewerApplicationOptions @@ -309,46 +309,94 @@ class LateXWorkshopPdfViewer implements ILatexWorkshopPdfViewer { this.sendCurrentStateToPanelManager() } - private forwardSynctex(position: { page: number, x: number, y: number, indicator: boolean }) { - if (!this.synctexEnabled) { - this.addLogMessage('SyncTeX temporarily disabled.') - return - } - // use the offsetTop of the actual page, much more accurate than multiplying the offsetHeight of the first page - // https://github.com/James-Yu/LaTeX-Workshop/pull/417 + private scrollToPosition(page: HTMLElement, posX: number, posY: number, isCircle: boolean = false): { scrollX: number, scrollY: number } { const container = document.getElementById('viewerContainer') as HTMLElement - const pos = PDFViewerApplication.pdfViewer._pages[position.page - 1].viewport.convertToViewportPoint(position.x, position.y) - const page = document.getElementsByClassName('page')[position.page - 1] as HTMLElement - const maxScrollX = window.innerWidth * 0.9 - const minScrollX = window.innerWidth * 0.1 - let scrollX = page.offsetLeft + pos[0] + const maxScrollX = window.innerWidth * (isCircle ? 0.9 : 1) + const minScrollX = window.innerWidth * (isCircle ? 0.1 : 0) + let scrollX = page.offsetLeft + posX scrollX = Math.min(scrollX, maxScrollX) scrollX = Math.max(scrollX, minScrollX) - const scrollY = page.offsetTop + page.offsetHeight - pos[1] + const scrollY = page.offsetTop + page.offsetHeight - posY - // set positions before and after SyncTeX to viewerHistory this.viewerHistory.set(container.scrollTop) if (PDFViewerApplication.pdfViewer.scrollMode === 1) { - // horizontal scrolling container.scrollLeft = page.offsetLeft } else { - // vertical scrolling container.scrollTop = scrollY - document.body.offsetHeight * 0.4 } this.viewerHistory.set(container.scrollTop) - if (!position.indicator) { - return + return { scrollX, scrollY } + } + + private createIndicator(type: 'rect' | 'circ', scrollX: number, scrollY: number, width_px?: number, height_px?: number): void { + let indicator = document.getElementById('synctex-indicator') as HTMLElement + + if (type === 'rect') { + const parent = indicator.parentNode as HTMLElement + indicator = indicator.cloneNode(true) as HTMLElement + indicator.id = '' + indicator.classList.add('synctex-indicator-rect') + indicator.style.width = `${width_px}px` + indicator.style.height = `${height_px}px` + indicator.addEventListener('animationend', () => { + indicator.style.display = 'none' + parent.removeChild(indicator) + }) + parent.appendChild(indicator) + } else { + indicator.className = 'show' + setTimeout(() => indicator.className = 'hide', 10) + // setTimeout(() => { + // indicator.style.left = '' + // indicator.style.top = '' + // }, 1000) } - const indicator = document.getElementById('synctex-indicator') as HTMLElement - indicator.className = 'show' indicator.style.left = `${scrollX}px` indicator.style.top = `${scrollY}px` - setTimeout(() => indicator.className = 'hide', 10) - // setTimeout(() => { - // indicator.style.left = '' - // indicator.style.top = '' - // }, 1000) + + } + + private forwardSynctexRect(data: SynctexRangeData[]) { + for (const record of data) { + const page = document.getElementsByClassName('page')[record.page - 1] as HTMLElement + const pos_left_top = PDFViewerApplication.pdfViewer._pages[record.page - 1].viewport.convertToViewportPoint(record.h, record.v - record.H) + const pos_right_down = PDFViewerApplication.pdfViewer._pages[record.page - 1].viewport.convertToViewportPoint(record.h + record.W, record.v) + + const { scrollX, scrollY } = this.scrollToPosition(page, pos_left_top[0], pos_left_top[1]) + + if (record.indicator) { + const width_px = pos_right_down[0] - pos_left_top[0] + const height_px = pos_left_top[1] - pos_right_down[1] + this.createIndicator('rect', scrollX, scrollY, width_px, height_px) + } + } + } + + private forwardSynctexCirc(data: SynctexData) { + const page = document.getElementsByClassName('page')[data.page - 1] as HTMLElement + // use the offsetTop of the actual page, much more accurate than multiplying the offsetHeight of the first page + // https://github.com/James-Yu/LaTeX-Workshop/pull/417 + const pos = PDFViewerApplication.pdfViewer._pages[data.page - 1].viewport.convertToViewportPoint(data.x, data.y) + const { scrollX, scrollY } = this.scrollToPosition(page, pos[0], pos[1], true) + + if (data.indicator) { + this.createIndicator('circ', scrollX, scrollY) + } + } + + private forwardSynctex(data: SynctexData | SynctexRangeData[]) { + if (!this.synctexEnabled) { + this.addLogMessage('SyncTeX temporarily disabled.') + return + } + + // if the type of data is SynctexRangeData[], parse as a rectangular indicator. + if (Array.isArray(data)){ + this.forwardSynctexRect(data) + } else { + this.forwardSynctexCirc(data) + } } private reload() {