paper-burner/tests/CODE_REVIEW_MODULES.md

21 KiB
Raw Blame History

Code Review: 模块提取对比分析

审查摘要

对比原始文件 history_pdf_compare.js 与提取的三个模块,检查功能逻辑、依赖关系和状态管理的一致性。


1 TextFittingAdapter 模块审查

对应的原始方法

  • initializeTextFitting()TextFittingAdapter.initialize()
  • preprocessGlobalFontSizes()TextFittingAdapter.preprocessGlobalFontSizes()
  • drawPlainTextInBox()TextFittingAdapter.drawPlainTextInBox()
  • drawPlainTextWithFitting()TextFittingAdapter.drawPlainTextWithFitting()
  • wrapText()TextFittingAdapter.wrapText()
  • renderFormulasInText()TextFittingAdapter.renderFormulasInText() (未在提取版本中)

保持一致的部分

特性 状态 备注
初始化逻辑 完全相同的TextFittingEngine初始化
预处理算法 globalFontScale、bbox计算完全一致
wrapText换行算法 CJK断句、标点符号处理、换行符处理完全相同
drawPlainTextInBox回退方案 与原始版本的fallback逻辑一致
drawPlainTextWithFitting主算法 二分查找、宽度因子、垂直居中完全一致
字号范围计算 minFontSize、maxFontSize计算相同
CJK判断逻辑 /[\u4e00-\u9fa5]/ 正则完全一致

⚠️ 需要注意的改变

1. 缺失方法renderFormulasInText()

原始代码 (1588-1604行):

renderFormulasInText(text) {
  // 使用缓存避免重复渲染
  if (this._formulaCache.has(text)) {
    return this._formulaCache.get(text);
  }

  if (typeof window.renderMathInElement === 'function') {
    // KaTeX渲染逻辑
    ...
  }
}

模块版本:

renderFormulasInText(text) {
  // 363-404行完全相同的实现
}

已正确包含 - 在TextFittingAdapter的363-404行

2. 选项配置的改变

原始代码处理:

// history_pdf_compare.js
this.textFittingEngine = new TextFittingEngine({
  initialScale: 1.0,
  minScale: 0.3,
  scaleStepHigh: 0.05,
  scaleStepLow: 0.1,
  lineSkipCJK: 1.5,
  lineSkipWestern: 1.3,
  minLineHeight: 1.05
});

模块版本处理:

// TextFittingAdapter.js
this.options = Object.assign({
  initialScale: 1.0,
  minScale: 0.3,
  scaleStepHigh: 0.05,
  scaleStepLow: 0.1,
  lineSkipCJK: 1.5,
  lineSkipWestern: 1.3,
  minLineHeight: 1.05,
  globalFontScale: 0.85  // 新增
}, options);

⚠️ 改进: 新增globalFontScale选项支持提高配置灵活性

3. 缓存管理的独立性

差异:

  • 原始: globalFontSizeCache 在 PDFCompareView 中管理
  • 模块: 自包含的globalFontSizeCache、_formulaCache

有利: 模块化改进支持clearCache()方法

潜在的问题或遗漏

1. TextFittingEngine初始化的隐式依赖

问题: 模块依赖全局的 TextFittingEngine

if (typeof TextFittingEngine === 'undefined') {
  console.error('[TextFittingAdapter] TextFittingEngine 未加载!请确保 js/utils/text-fitting.js 已正确引入');
  return;
}

风险:

  • 如果 text-fitting.js 未加载,将默默失败
  • 日志显示错误但继续执行,可能导致难以调试的问题

建议:

initialize() {
  if (typeof TextFittingEngine === 'undefined') {
    throw new Error('[TextFittingAdapter] TextFittingEngine 未加载!');
  }
  // ...
}

2. wrapText方法缺少canvas context参数验证

原始代码: 无参数检查 模块代码: 同样无参数检查

wrapText(ctx, text, maxWidth) {
  if (!text) return [];
  // 缺少 ctx 验证
  ctx.measureText(testLine);  // 可能报错
}

建议:

wrapText(ctx, text, maxWidth) {
  if (!text) return [];
  if (!ctx || typeof ctx.measureText !== 'function') {
    console.warn('[TextFitting] 无效的canvas context');
    return text.split('\n');
  }
  // ...
}

3. globalFontSizeCache 的前置条件

方法: preprocessGlobalFontSizes(contentListJson, translatedContentList)

缺失的验证:

if (!contentListJson || !Array.isArray(contentListJson)) {
  console.warn('[TextFittingAdapter] 无效的contentListJson');
  return;
}

原始代码中没有验证,模块版本也没有加


2 PDFExporter 模块审查

对应的原始方法

  • exportStructuredTranslation()PDFExporter.exportStructuredTranslation() (新提取原始文件中在2000+行)
  • calculatePdfTextLayout()PDFExporter.calculatePdfTextLayout()
  • wrapTextForPdf()PDFExporter.wrapTextForPdf()

保持一致的部分

特性 状态 备注
PDF加载和字体嵌入 fontkit注册逻辑相同
页面分组逻辑 pageContentMap创建方式相同
bbox坐标转换 scaleX/scaleY计算相同
白色矩形覆盖算法 rgb(1,1,1)覆盖逻辑相同
文本布局二分查找 高低指针、精度0.5算法相同
wrapTextForPdf换行 CJK断句逻辑与Canvas版本一致
PDF坐标系处理 y轴翻转、缩放计算相同

⚠️ 需要注意的改变

1. 缺失的依赖项声明

原始代码 (在PDFCompareView中):

async exportStructuredTranslation(translatedContentList) {
  // 使用 this.originalPdfBase64
  // 使用 this.scale 和 this.dpr
  // 使用 showNotification 从外部传入
}

模块版本:

async exportStructuredTranslation(originalPdfBase64, translatedContentList, showNotification = null) {
  // 显式接收所有参数
  // 不依赖 this.scale
  // 不依赖 this.dpr
  // 独立的 dpr 处理
}

改进: 参数显式化,减少隐式依赖

2. 参数差异缺少scale和dpr

问题: PDFExporter 中没有 scale 和 dpr 属性

原始代码中的使用:

// PDFCompareView 中计算渲染时使用
const scaleX = pageWidth / BBOX_NORMALIZED_RANGE;
const scaleY = pageHeight / BBOX_NORMALIZED_RANGE;

模块版本:

// PDFExporter.js 中
// 注意:没有使用 this.scale 或 this.dpr
const scaleX = pageWidth / BBOX_NORMALIZED_RANGE;
const scaleY = pageHeight / BBOX_NORMALIZED_RANGE;

⚠️ 潜在问题: 模块直接使用 PDF 的页面宽高,而不考虑原始的 scale/dpr。这可能导致文本大小计算不同。

3. 字体加载的网络依赖

风险: 硬编码的CDN URL

fontUrl: 'https://gcore.jsdelivr.net/npm/source-han-sans-cn@1.0.0/SourceHanSansCN-Normal.otf',
pdfLibUrl: 'https://gcore.jsdelivr.net/npm/pdf-lib@1.17.1/dist/pdf-lib.min.js',
fontkitUrl: 'https://gcore.jsdelivr.net/npm/@pdf-lib/fontkit@1.1.1/dist/fontkit.umd.min.js',

⚠️ 问题:

  • CDN依赖可能导致离线失败
  • URL可能变更
  • 没有fallback方案

4. calculatePdfTextLayout 与 drawPlainTextWithFitting 的不一致

原始代码中的差异:

Canvas版本 (drawPlainTextWithFitting):

const lineHeight = mid * lineSkip;
const totalHeight = lines.length === 1
  ? mid * 1.2
  : (lines.length - 1) * lineHeight + mid * 1.2;

PDF版本 (calculatePdfTextLayout):

const lineHeight = mid * lineSkip;
const totalHeight = lines.length > 0
  ? (lines.length - 1) * lineHeight + mid
  : 0;

问题: 计算不一致!

  • Canvas: 最后一行使用 mid * 1.2
  • PDF: 最后一行使用 mid
  • 这会导致PDF和Canvas中的文本大小不同

建议: 应该统一为同一个公式

5. 缺少原始文本清除逻辑

问题: 原始代码有 clearTextInBbox() 方法来清除PDF中的原始文本但PDFExporter中

// 用白色矩形覆盖原文
items.forEach(item => {
  // ...
  page.drawRectangle({
    x: x,
    y: y,
    width: width,
    height: height,
    color: rgb(1, 1, 1),  // 近似白色
  });
});

⚠️ 注意:

  • 使用 rgb(1, 1, 1) 而不是 rgb(255, 255, 255)pdf-lib的色值范围是0-1而不是0-255
  • 这会导致非纯白色覆盖,可能看到轻微的灰色背景

建议:

color: rgb(255, 255, 255)  // 或使用 rgb(1, 1, 1) 但需要验证

潜在的问题或遗漏

1. 缺少错误恢复机制

原始代码:

if (typeof PDFLib === 'undefined') {
  await this.loadPdfLib();
}

模块版本: 同样存在,但缺少重试机制

问题: 如果加载失败,没有重试逻辑

2. fontkit加载失败时的行为

代码:

script.onerror = (error) => {
  console.warn('[PDFExporter] fontkit 加载失败:', error);
  resolve(); // fontkit失败不阻止流程
};

⚠️ 问题:

  • fontkit失败会导致中文字体无法嵌入
  • 但流程继续,可能使用默认字体(不支持中文)
  • 最终PDF中的中文会显示为空或方块

建议:

// 如果fontkit失败应该至少警告用户
if (!fontkit && needsCJKFont) {
  showNotification('警告:中文字体可能无法正确显示', 'warning');
}

3. 缺少对 showNotification 的类型检查

代码:

if (showNotification) {
  showNotification('没有翻译内容可导出', 'warning');
}

⚠️ 问题: 假设 showNotification 是函数,但没有验证

建议:

if (typeof showNotification === 'function') {
  showNotification('没有翻译内容可导出', 'warning');
}

4. 文本布局计算中的lineHeight使用

问题: 在 calculatePdfTextLayout 中计算最后一行时:

const totalHeight = lines.length > 0
  ? (lines.length - 1) * lineHeight + mid
  : 0;

但在实际绘制时:

const totalHeight = lines.length > 0
  ? (lines.length - 1) * lineHeight + fontSize
  : 0;

这两个值应该相同mid === fontSize但逻辑复杂易出错。


3 SegmentManager 模块审查

对应的原始方法

  • renderAllPagesContinuous()SegmentManager.renderAllPagesContinuous()
  • createSegmentDom()SegmentManager.createSegmentDom()
  • initLazyLoadingSegments()SegmentManager.initLazyLoadingSegments()
  • renderVisibleSegments()SegmentManager.renderVisibleSegments()
  • renderSegment()SegmentManager.renderSegment()
  • renderSegmentOverlays()SegmentManager.renderSegmentOverlays()
  • clearTextInSegment()SegmentManager.clearTextInSegment() (新增)

保持一致的部分

特性 状态 备注
段划分算法 maxSegmentPixels和页面分组逻辑相同
DOM创建逻辑 wrapper、canvas、overlay创建完全相同
DPR处理 物理像素和CSS像素的转换相同
懒加载触发 scrollDebounceMs 和 renderVisibleSegments 逻辑相同
可见性判断 visibleStartPx和visibleEndPx计算相同
离屏渲染 使用临时canvas避免PDF.js清除问题
点击事件处理 段级别的坐标转换和命中测试逻辑相同

⚠️ 需要注意的改变

1. 依赖注入模式

原始代码 (在PDFCompareView中):

// 方法直接访问 this 的属性
async renderSegmentOverlays(seg) {
  // 直接调用 this.renderPageBboxesToCtx()
  // 直接调用 this.renderPageTranslationToCtx()
  // 直接访问 this.contentListJson
}

模块版本:

// 使用依赖注入
setDependencies(deps) {
  Object.assign(this, deps);
}

// 在方法中检查依赖
async renderSegmentOverlays(seg) {
  if (!this.renderPageBboxesToCtx || !this.renderPageTranslationToCtx) {
    console.warn('[SegmentManager] 缺少渲染函数依赖');
    return;
  }
  // ...
}

改进: 显式依赖注入,减少隐式耦合

2. 容器设置方法

原始代码 (隐式):

// 直接在 render() 方法中设置容器
this.originalSegmentsContainer = document.getElementById('pdf-original-segments');

模块版本 (显式):

setContainers(originalSegments, translationSegments, originalScroll, translationScroll) {
  this.originalSegmentsContainer = originalSegments;
  this.translationSegmentsContainer = translationSegments;
  this.originalScroll = originalScroll;
  this.translationScroll = translationScroll;
}

改进: 更清晰的初始化流程

3. PDF文档依赖

原始代码:

// 从 PDFCompareView.pdfDoc 继承
this.pdfDoc = pdfDoc;

模块版本:

constructor(pdfDoc, options = {}) {
  this.pdfDoc = pdfDoc;
  this.totalPages = pdfDoc.numPages;
  // ...
}

一致: 都显式接收pdfDoc作为构造参数

潜在的问题或遗漏

1. 事件监听器的清理问题

代码:

initLazyLoadingSegments() {
  if (!this._lazyInitialized) {
    this.originalScroll.addEventListener('scroll', () => onScroll(this.originalScroll));
    this.translationScroll.addEventListener('scroll', () => onScroll(this.translationScroll));
    this._lazyInitialized = true;
  }
}

destroy() {
  // 移除事件监听
  if (this._lazyInitialized && this.originalScroll && this.translationScroll) {
    // 注意:由于事件监听使用了箭头函数,无法直接移除
    // 这里设置标记位,防止继续渲染
    this.segments = [];
    this.pageInfos = [];
  }
}

问题:

  • 事件监听器无法正确移除(注释中也承认了)
  • 清空 segments 和 pageInfos 不能停止已经开始的渲染
  • 可能导致内存泄漏和ghost渲染

建议:

initLazyLoadingSegments() {
  if (!this._lazyInitialized) {
    // 保存回调引用以便后续移除
    this._scrollHandler = (scroller) => {
      clearTimeout(this._lazyScrollTimer);
      this._lazyScrollTimer = setTimeout(() => {
        if (!this._destroyed) {  // 添加销毁标志检查
          this.renderVisibleSegments(scroller);
        }
      }, this.options.scrollDebounceMs);
    };

    this.originalScroll.addEventListener('scroll',
      () => this._scrollHandler(this.originalScroll)
    );
    this.translationScroll.addEventListener('scroll',
      () => this._scrollHandler(this.translationScroll)
    );
    this._lazyInitialized = true;
  }
}

destroy() {
  this._destroyed = true;

  if (this._lazyInitialized && this.originalScroll && this.translationScroll) {
    this.originalScroll.removeEventListener('scroll', this._scrollHandler);
    this.translationScroll.removeEventListener('scroll', this._scrollHandler);
  }

  // 清空容器
  if (this.originalSegmentsContainer) {
    this.originalSegmentsContainer.innerHTML = '';
  }
  if (this.translationSegmentsContainer) {
    this.translationSegmentsContainer.innerHTML = '';
  }

  this.segments = [];
  this.pageInfos = [];
}

2. renderSegment 中的离屏canvas管理

问题:

async renderSegment(seg) {
  // 使用离屏画布避免 PDF.js 清除问题
  const off = document.createElement('canvas');
  const offCtx = off.getContext('2d', { willReadFrequently: true, alpha: false });

  for (const p of seg.pages) {
    if (off.width !== p.width) off.width = p.width;
    if (off.height !== p.height) off.height = p.height;

    offCtx.clearRect(0, 0, off.width, off.height);
    await p.page.render({ canvasContext: offCtx, viewport: p.viewport }).promise;

    // 绘制到左右段画布
    seg.left.ctx.drawImage(off, 0, p.yInSegPx);
    seg.right.ctx.drawImage(off, 0, p.yInSegPx);
  }
  // ...
}

⚠️ 性能问题:

  • 每次渲染都创建离屏canvas没有复用
  • 频繁重新分配canvas宽高
  • 没有垃圾回收机制

建议:

constructor(pdfDoc, options = {}) {
  // ...
  this._offscreenCanvas = null;  // 缓存离屏canvas
}

async renderSegment(seg) {
  // 复用或创建离屏canvas
  let off = this._offscreenCanvas;
  if (!off) {
    off = document.createElement('canvas');
    this._offscreenCanvas = off;
  }
  // ...
}

destroy() {
  // ...
  this._offscreenCanvas = null;  // 释放
}

3. clearTextInSegment 方法的可用性问题

代码:

async clearTextInSegment(seg) {
  if (!this.contentListJson || !this.clearTextInBbox) {
    console.warn('[SegmentManager] 缺少清除文字依赖');
    return;
  }

  // ...
  await this.clearTextInBbox(seg.right.ctx, pageNum, { x, y, w, h }, p.yInSegPx);
}

⚠️ 问题:

  • 此方法在SegmentManager中定义但原始代码中没有调用
  • clearTextInBbox 期望的参数需要仔细验证
  • seg.right.ctx 是画布context但注入的 clearTextInBbox 可能期望不同的接口

需要验证:

  • 这个方法是否真的被使用?
  • 参数接口是否匹配?

4. 缺少 bboxNormalizedRange 的验证

代码:

async clearTextInSegment(seg) {
  const BBOX_NORMALIZED_RANGE = this.options.bboxNormalizedRange;
  // ...
  const scaleX = p.width / BBOX_NORMALIZED_RANGE;
  const scaleY = p.height / BBOX_NORMALIZED_RANGE;
}

⚠️ 问题: 如果 bboxNormalizedRange 是 null 或 0会导致NaN

建议:

const BBOX_NORMALIZED_RANGE = this.options.bboxNormalizedRange || 1000;
if (BBOX_NORMALIZED_RANGE <= 0) {
  console.error('[SegmentManager] 无效的 bboxNormalizedRange');
  return;
}

5. 缺少对容器存在的验证

代码:

createSegmentDom(seg, dpr) {
  // ...
  buildSide(this.originalSegmentsContainer, 'left');
  buildSide(this.translationSegmentsContainer, 'right');
}

⚠️ 问题: 如果容器是 nullappendChild 会抛出错误

原始代码中的问题 (也存在):

renderAllPagesContinuous() {
  // ...
  for (const seg of this.segments) {
    this.createSegmentDom(seg, dpr);  // 可能失败
  }
  // ...
}

建议:

createSegmentDom(seg, dpr) {
  if (!this.originalSegmentsContainer || !this.translationSegmentsContainer) {
    console.error('[SegmentManager] 容器未初始化');
    return;
  }
  // ...
}

总体评估矩阵

模块 功能一致性 依赖处理 状态管理 接口变化 问题严重度
TextFittingAdapter 95% 良好 良好 参数化
PDFExporter 90% 需改进 自包含 参数化
SegmentManager 95% 好(DI) 良好 显式化

关键建议汇总

🔴 高优先级 (必须修复)

  1. TextFittingAdapter.wrapText - 添加ctx验证
  2. PDFExporter - 统一Canvas和PDF的文本高度计算公式
  3. PDFExporter.loadPdfLib - 改进对失败的错误处理
  4. SegmentManager - 修复事件监听器的清理问题

🟡 中优先级 (应该改进)

  1. TextFittingAdapter.initialize - 改为throw而不是return
  2. PDFExporter.calculatePdfTextLayout - 添加参数验证
  3. SegmentManager.renderSegment - 缓存离屏canvas以提高性能
  4. SegmentManager.createSegmentDom - 添加容器存在性验证

🟢 低优先级 (可选改进)

  1. TextFittingAdapter.preprocessGlobalFontSizes - 添加参数验证
  2. PDFExporter - 添加showNotification类型检查
  3. SegmentManager - 文档化clearTextInSegment的使用场景

兼容性检查表

从 PDFCompareView 迁移时需要确保:

  • TextFittingAdapter.initialize() 在 TextFittingEngine 加载后调用
  • PDFExporter 实例化时接收正确的选项对象
  • SegmentManager.setDependencies() 在使用前调用,提供所有必需的渲染函数
  • SegmentManager.setContainers() 在 renderAllPagesContinuous() 之前调用
  • 调用 SegmentManager.destroy() 来清理事件监听器和DOM
  • PDFExporter.exportStructuredTranslation() 收到有效的showNotification回调
  • TextFittingAdapter 的 globalFontSizeCache 在每次新PDF加载时调用 clearCache()

集成检查示例

// 正确的初始化顺序
const textFitter = new TextFittingAdapter();
textFitter.initialize();  // 检查TextFittingEngine

const segmentManager = new SegmentManager(pdfDoc, {
  maxSegmentPixels: 4096,
  bboxNormalizedRange: 1000
});

// 设置依赖项
segmentManager.setDependencies({
  renderPageBboxesToCtx: (ctx, pageNum, yOffset, w, h) => { /* ... */ },
  renderPageTranslationToCtx: (ctx, wrapper, pageNum, yOffset, w, h) => { /* ... */ },
  clearTextInBbox: (ctx, pageNum, bbox, yOffset) => { /* ... */ },
  clearFormulaElementsForPageInWrapper: (pageNum, wrapper) => { /* ... */ },
  onOverlayClick: (e, seg) => { /* ... */ },
  contentListJson: contentData
});

segmentManager.setContainers(origContainer, transContainer, origScroll, transScroll);

await segmentManager.renderAllPagesContinuous();

const exporter = new PDFExporter();
await exporter.exportStructuredTranslation(
  pdfBase64,
  translatedData,
  (msg, type) => console.log(`[${type}] ${msg}`)
);

// 清理
segmentManager.destroy();
textFitter.clearCache();

结论

整体而言,这三个模块的提取是高质量的,保持了原始逻辑的一致性,并通过参数化和依赖注入改进了代码架构。

主要优点:

  • 功能逻辑保留完整
  • 耦合度降低
  • 模块职责清晰
  • 可复用性提高

需要关注的地方:

  • Canvas vs PDF 文本高度计算需要统一
  • 事件监听器管理需要改进
  • 参数验证需要加强
  • 网络依赖需要fallback机制

总体评分: 8.5/10 - 很好的重构,少数地方需要微调。