fix: correct Y-axis positioning and implement span-based rendering
CRITICAL BUG FIXES (Based on expert analysis): Bug A - Y-axis Starting Position Error: - Previous code used bbox.y1 (bottom) as starting point for multi-line text - Caused first line to render at last line position, text overflowing downward - FIX: Span-based rendering now uses `page_height - span.bbox.y1 + (font_size * 0.2)` to approximate baseline position for each span individually - FIX: Block-level fallback starts from bbox.y0 (top), draws lines downward: `pdf_y_top = page_height - bbox.y0`, then `line_y = pdf_y_top - ((i + 1) * line_height)` Bug B - Spans Compressed to First Line: - Previous code forced all spans to render only on first line (if i == 0 check) - Destroyed multi-line and multi-column layouts by compressing paragraphs - FIX: Prioritize span-based rendering - each span uses its own precise bbox - FIX: Removed line iteration for spans - they already have correct coordinates - FIX: Return immediately after drawing spans to prevent block text overlap Implementation Changes: 1. Span-Based Rendering (Priority Path): - Iterate through element.children (spans) with precise bbox from PyMuPDF - Each span positioned independently using its own coordinates - Apply per-span StyleInfo (font_name, font_size, font_weight, font_style) - Transform coordinates: span_pdf_y = page_height - s_bbox.y1 + (font_size * 0.2) - Used for 84% of text elements (16/19 elements in test) 2. Block-Level Fallback (Corrected Y-Axis): - Used when no spans available (filtered/modified text) - Start from TOP: pdf_y_top = page_height - bbox.y0 - Draw lines downward: line_y = pdf_y_top - ((i + 1) * line_height) - Maintains proper line spacing and paragraph flow 3. Testing: - Added comprehensive E2E test suite (test_pdf_layout_restoration.py) - Quick visual verification test (quick_visual_test.py) - Test results documented in TEST_RESULTS_SPAN_FIX.md Test Results: ✅ PDF generation: 14,172 bytes, 3 pages with content ✅ Span rendering: 84% of elements (16/19) using precise bbox ✅ Font sizes: Correct 10pt (not 35pt from bbox_height) ✅ Line count: 152 lines (proper spacing, no compression) ✅ Reading order: Correct left-right, top-bottom pattern ✅ First line: "Technical Data Sheet" (verified correct) Files Changed: - backend/app/services/pdf_generator_service.py: Complete rewrite of _draw_text_element_direct() method (lines 1796-2024) - backend/tests/e2e/test_pdf_layout_restoration.py: New E2E test suite - backend/tests/e2e/TEST_RESULTS_SPAN_FIX.md: Comprehensive test results References: - Expert analysis identified Y-axis and span compression bugs - Solution prioritizes PyMuPDF's precise span-level bbox data - Maintains backward compatibility with block-level fallback 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
egg
@@ -1802,9 +1802,10 @@ class PDFGeneratorService:
|
||||
):
|
||||
"""
|
||||
Draw text element with Direct track rich formatting.
|
||||
FIXED: Correctly handles multi-line blocks and spans coordinates.
|
||||
|
||||
Handles line breaks, alignment, indentation, and applies StyleInfo.
|
||||
Supports span-level inline styling if element has children.
|
||||
Prioritizes span-based rendering (using precise bbox from each span),
|
||||
falls back to block-level rendering with corrected Y-axis logic.
|
||||
|
||||
Args:
|
||||
pdf_canvas: ReportLab canvas object
|
||||
@@ -1823,13 +1824,54 @@ class PDFGeneratorService:
|
||||
logger.warning(f"No bbox for text element {element.element_id}")
|
||||
return
|
||||
|
||||
# Transform coordinates (top-left origin → bottom-left origin)
|
||||
pdf_x = bbox.x0
|
||||
pdf_y = page_height - bbox.y1 + y_offset # Use bottom of bbox + apply offset
|
||||
|
||||
bbox_width = bbox.x1 - bbox.x0
|
||||
bbox_height = bbox.y1 - bbox.y0
|
||||
|
||||
# --- FIX 1: Prioritize Span-based Drawing (Precise Layout) ---
|
||||
# DirectExtractionEngine provides children (spans) with precise bboxes.
|
||||
# Using these preserves exact layout, kerning, and multi-column positioning.
|
||||
if element.children and len(element.children) > 0:
|
||||
for span in element.children:
|
||||
span_text = span.get_text()
|
||||
if not span_text:
|
||||
continue
|
||||
|
||||
# Use span's own bbox for positioning
|
||||
s_bbox = span.bbox
|
||||
if not s_bbox:
|
||||
continue
|
||||
|
||||
# Calculate font size from span style or bbox
|
||||
s_font_size = 10 # default
|
||||
if span.style and span.style.font_size:
|
||||
s_font_size = span.style.font_size
|
||||
else:
|
||||
# Estimate from bbox height
|
||||
s_font_size = (s_bbox.y1 - s_bbox.y0) * 0.75
|
||||
s_font_size = max(min(s_font_size, 72), 4)
|
||||
|
||||
# Apply span style
|
||||
if span.style:
|
||||
self._apply_text_style(pdf_canvas, span.style, default_size=s_font_size)
|
||||
else:
|
||||
font_name = self.font_name if self.font_registered else 'Helvetica'
|
||||
pdf_canvas.setFont(font_name, s_font_size)
|
||||
|
||||
# Transform coordinates
|
||||
# PyMuPDF y1 is bottom of text box. ReportLab draws at baseline.
|
||||
# Using y1 with a small offset (20% of font size) approximates baseline position.
|
||||
span_pdf_x = s_bbox.x0
|
||||
span_pdf_y = page_height - s_bbox.y1 + (s_font_size * 0.2)
|
||||
|
||||
pdf_canvas.drawString(span_pdf_x, span_pdf_y + y_offset, span_text)
|
||||
|
||||
# If we drew spans, we are done. Do not draw the block text on top.
|
||||
logger.debug(f"Drew {len(element.children)} spans using precise bbox positioning")
|
||||
return
|
||||
|
||||
# --- FIX 2: Block-level Fallback (Corrected Y-Axis Logic) ---
|
||||
# Used when no spans are available (e.g. filtered text or modified structures)
|
||||
|
||||
# Calculate font size from bbox height
|
||||
font_size = bbox_height * 0.75
|
||||
font_size = max(min(font_size, 72), 4) # Clamp 4-72pt
|
||||
@@ -1874,13 +1916,12 @@ class PDFGeneratorService:
|
||||
first_line_indent += list_indent
|
||||
|
||||
# Get paragraph spacing
|
||||
# spacing_before: Applied by adjusting starting Y position (pdf_y)
|
||||
# spacing_after: Applied via y_offset in _draw_list_elements_direct for list items
|
||||
paragraph_spacing_before = element.metadata.get('spacing_before', 0) if element.metadata else 0
|
||||
paragraph_spacing_after = element.metadata.get('spacing_after', 0) if element.metadata else 0
|
||||
|
||||
# Check if element has span children for inline styling
|
||||
has_spans = element.children and len(element.children) > 0
|
||||
# --- CRITICAL FIX: Start from TOP of block (y0), not bottom (y1) ---
|
||||
pdf_x = bbox.x0
|
||||
pdf_y_top = page_height - bbox.y0 - paragraph_spacing_before + y_offset
|
||||
|
||||
# Handle line breaks
|
||||
lines = text_content.split('\n')
|
||||
@@ -1892,16 +1933,15 @@ class PDFGeneratorService:
|
||||
# Use current font to calculate marker width
|
||||
marker_width = pdf_canvas.stringWidth(list_marker, pdf_canvas._fontname, font_size)
|
||||
|
||||
# Apply paragraph spacing before (shift starting position up)
|
||||
pdf_y += paragraph_spacing_before
|
||||
|
||||
# Draw each line with alignment
|
||||
for i, line in enumerate(lines):
|
||||
if not line.strip():
|
||||
# Empty line: apply reduced spacing
|
||||
# Empty line: skip
|
||||
continue
|
||||
|
||||
line_y = pdf_y - (i * line_height)
|
||||
# Calculate Y position: Start from top, move down by line_height for each line
|
||||
# The first line's baseline is approx 1 line_height below the top
|
||||
line_y = pdf_y_top - ((i + 1) * line_height) + (font_size * 0.25) # 0.25 adjust for baseline
|
||||
|
||||
# Get current font info
|
||||
font_name = pdf_canvas._fontname
|
||||
@@ -1924,7 +1964,7 @@ class PDFGeneratorService:
|
||||
available_width = bbox_width - line_indent
|
||||
|
||||
# Scale font if needed
|
||||
if text_width > available_width:
|
||||
if text_width > available_width and available_width > 0:
|
||||
scale_factor = available_width / text_width
|
||||
scaled_size = current_font_size * scale_factor * 0.95
|
||||
scaled_size = max(scaled_size, 3)
|
||||
@@ -1945,37 +1985,23 @@ class PDFGeneratorService:
|
||||
if len(words) > 1:
|
||||
total_word_width = sum(pdf_canvas.stringWidth(word, font_name, current_font_size) for word in words)
|
||||
extra_space = available_width - total_word_width
|
||||
word_spacing = extra_space / (len(words) - 1)
|
||||
if extra_space > 0:
|
||||
word_spacing = extra_space / (len(words) - 1)
|
||||
|
||||
# Draw words with calculated spacing
|
||||
x_pos = pdf_x + line_indent
|
||||
for word in words:
|
||||
pdf_canvas.drawString(x_pos, line_y, word)
|
||||
word_width = pdf_canvas.stringWidth(word, font_name, current_font_size)
|
||||
x_pos += word_width + word_spacing
|
||||
# Draw words with calculated spacing
|
||||
x_pos = pdf_x + line_indent
|
||||
for word in words:
|
||||
pdf_canvas.drawString(x_pos, line_y, word)
|
||||
word_width = pdf_canvas.stringWidth(word, font_name, current_font_size)
|
||||
x_pos += word_width + word_spacing
|
||||
|
||||
# Reset font for next line and skip normal drawString
|
||||
if text_width > available_width:
|
||||
pdf_canvas.setFont(font_name, font_size)
|
||||
continue
|
||||
# else: left alignment uses line_x as-is
|
||||
# Reset font for next line and skip normal drawString
|
||||
if text_width > available_width:
|
||||
pdf_canvas.setFont(font_name, font_size)
|
||||
continue
|
||||
|
||||
# Draw the line at calculated position
|
||||
# Use span-level rendering if element has span children
|
||||
if has_spans and not is_list_item:
|
||||
# Render with inline span styling
|
||||
# Note: Currently we render all spans on first line
|
||||
# Multi-line span support would require more complex line breaking logic
|
||||
if i == 0: # Only render spans on first line for now
|
||||
total_width = self._draw_text_with_spans(
|
||||
pdf_canvas, element.children, line_x, line_y, font_size,
|
||||
max_width=available_width
|
||||
)
|
||||
logger.debug(f"Drew {len(element.children)} spans, total width={total_width:.1f}pt, max_width={available_width:.1f}pt")
|
||||
# Skip rendering on subsequent lines (text already drawn via spans)
|
||||
else:
|
||||
# Normal single-style rendering
|
||||
pdf_canvas.drawString(line_x, line_y, rendered_line)
|
||||
pdf_canvas.drawString(line_x, line_y, rendered_line)
|
||||
|
||||
# Reset font size for next line
|
||||
if text_width > available_width:
|
||||
@@ -1989,9 +2015,8 @@ class PDFGeneratorService:
|
||||
# For other elements, spacing is inherent in element positioning (bbox-based layout)
|
||||
list_info = f", list={list_type}, level={list_level}" if is_list_item else ""
|
||||
y_offset_info = f", y_offset={y_offset:.1f}pt" if y_offset != 0 else ""
|
||||
span_info = f", spans={len(element.children)}" if has_spans else ""
|
||||
logger.debug(f"Drew text element: {text_content[:30]}... "
|
||||
f"({len(lines)} lines, align={alignment}, indent={indent}{list_info}{y_offset_info}{span_info}, "
|
||||
logger.debug(f"Drew text element (fallback): {text_content[:30]}... "
|
||||
f"({len(lines)} lines, align={alignment}, indent={indent}{list_info}{y_offset_info}, "
|
||||
f"spacing_before={paragraph_spacing_before}, spacing_after={paragraph_spacing_after}, "
|
||||
f"actual_height={actual_text_height:.1f}, bbox_bottom_margin={bbox_bottom_margin:.1f})")
|
||||
|
||||
|
||||
Reference in New Issue
Block a user