From 1730402effee0762a2499fc2516f64044785d321 Mon Sep 17 00:00:00 2001 From: Reece Browne Date: Mon, 18 Aug 2025 13:19:20 +0100 Subject: [PATCH] Fix and improve pageeditor --- .../components/pageEditor/DragDropGrid.tsx | 191 ++++++++++++++---- .../src/components/pageEditor/PageEditor.tsx | 107 ++++++---- .../components/pageEditor/PageThumbnail.tsx | 50 +++-- frontend/src/hooks/useThumbnailGeneration.ts | 176 +++++++++++++++- .../services/thumbnailGenerationService.ts | 15 +- 5 files changed, 434 insertions(+), 105 deletions(-) diff --git a/frontend/src/components/pageEditor/DragDropGrid.tsx b/frontend/src/components/pageEditor/DragDropGrid.tsx index 3639f74d9..3e9fd2206 100644 --- a/frontend/src/components/pageEditor/DragDropGrid.tsx +++ b/frontend/src/components/pageEditor/DragDropGrid.tsx @@ -1,4 +1,4 @@ -import React, { useState, useCallback, useRef, useEffect } from 'react'; +import React, { useState, useCallback, useRef, useEffect, useMemo } from 'react'; import { Box } from '@mantine/core'; import styles from './PageEditor.module.css'; @@ -47,6 +47,100 @@ const DragDropGrid = ({ dragPosition, }: DragDropGridProps) => { const itemRefs = useRef>(new Map()); + const containerRef = useRef(null); + const [scrollTop, setScrollTop] = useState(0); + + // Virtualization configuration - adjust for document size + const isLargeDocument = items.length > 1000; // Only virtualize for very large documents + const ITEM_HEIGHT = 340; // Height of PageThumbnail + gap (20rem + gap) + const ITEMS_PER_ROW = 4; // Approximate items per row + const BUFFER_SIZE = isLargeDocument ? 2 : 3; // Larger buffer for smoother scrolling + const OVERSCAN = ITEMS_PER_ROW * BUFFER_SIZE; // Total buffer items + + // Log virtualization stats for debugging + React.useEffect(() => { + if (items.length > 100) { + console.log(`📊 DragDropGrid: Virtualizing ${items.length} items (large doc: ${isLargeDocument}, buffer: ${BUFFER_SIZE})`); + } + }, [items.length, isLargeDocument, BUFFER_SIZE]); + + // Throttled scroll handler to prevent excessive re-renders + const throttleRef = useRef(); + + // Detect scroll position from parent container + useEffect(() => { + const updateScrollPosition = () => { + // Throttle scroll updates for better performance + if (throttleRef.current) { + cancelAnimationFrame(throttleRef.current); + } + + throttleRef.current = requestAnimationFrame(() => { + const scrollingParent = containerRef.current?.closest('[data-scrolling-container]') || + containerRef.current?.offsetParent?.closest('div[style*="overflow"]'); + + if (scrollingParent) { + setScrollTop(scrollingParent.scrollTop || 0); + } + }); + }; + + const scrollingParent = containerRef.current?.closest('[data-scrolling-container]') || + containerRef.current?.offsetParent?.closest('div[style*="overflow"]'); + + if (scrollingParent) { + // Use passive listener for better scrolling performance + scrollingParent.addEventListener('scroll', updateScrollPosition, { passive: true }); + updateScrollPosition(); // Initial position + + return () => { + scrollingParent.removeEventListener('scroll', updateScrollPosition); + if (throttleRef.current) { + cancelAnimationFrame(throttleRef.current); + } + }; + } + }, []); + + // Calculate visible range with virtualization (only for very large documents) + const { startIndex, endIndex, totalHeight, topSpacer } = useMemo(() => { + // Skip virtualization for smaller documents to avoid jankiness + if (!isLargeDocument) { + return { + startIndex: 0, + endIndex: items.length, + totalHeight: Math.ceil(items.length / ITEMS_PER_ROW) * ITEM_HEIGHT, + topSpacer: 0 + }; + } + + const containerHeight = containerRef.current?.clientHeight || 600; + const rowHeight = ITEM_HEIGHT; + const totalRows = Math.ceil(items.length / ITEMS_PER_ROW); + const visibleRows = Math.ceil(containerHeight / rowHeight); + + const startRow = Math.max(0, Math.floor(scrollTop / rowHeight) - BUFFER_SIZE); + const endRow = Math.min(totalRows, startRow + visibleRows + BUFFER_SIZE * 2); + + const startIndex = startRow * ITEMS_PER_ROW; + const endIndex = Math.min(items.length, endRow * ITEMS_PER_ROW); + const totalHeight = totalRows * rowHeight; + const topSpacer = startRow * rowHeight; + + return { startIndex, endIndex, totalHeight, topSpacer }; + }, [items.length, scrollTop, ITEM_HEIGHT, ITEMS_PER_ROW, BUFFER_SIZE, isLargeDocument]); + + // Only render visible items for performance + const visibleItems = useMemo(() => { + const visible = items.slice(startIndex, endIndex); + + // Debug logging for large documents + if (items.length > 500 && visible.length > 0) { + console.log(`📊 DragDropGrid: Rendering ${visible.length} items (${startIndex}-${endIndex-1}) of ${items.length} total`); + } + + return visible; + }, [items, startIndex, endIndex]); // Global drag cleanup useEffect(() => { @@ -70,49 +164,68 @@ const DragDropGrid = ({ }, [draggedItem, onDragEnd]); return ( - +
- {items.map((item, index) => ( - - {/* Split marker */} - {renderSplitMarker && item.splitBefore && index > 0 && renderSplitMarker(item, index)} + {/* Top spacer for virtualization */} +
+ + {/* Visible items container */} +
+ {visibleItems.map((item, visibleIndex) => { + const actualIndex = startIndex + visibleIndex; + return ( + + {/* Split marker */} + {renderSplitMarker && item.splitBefore && actualIndex > 0 && renderSplitMarker(item, actualIndex)} - {/* Item */} - {renderItem(item, index, itemRefs)} - - ))} - - {/* End drop zone */} -
-
onDrop(e, 'end')} - > -
- Drop here to
move to end + {/* Item */} + {renderItem(item, actualIndex, itemRefs)} + + ); + })} + + {/* End drop zone - inline with pages */} +
+
onDrop(e, 'end')} + > +
+ Drop here to
move to end +
diff --git a/frontend/src/components/pageEditor/PageEditor.tsx b/frontend/src/components/pageEditor/PageEditor.tsx index ad0125f92..99accb3f1 100644 --- a/frontend/src/components/pageEditor/PageEditor.tsx +++ b/frontend/src/components/pageEditor/PageEditor.tsx @@ -132,48 +132,79 @@ const PageEditor = ({ } console.log(`🎬 Will use ${(processedFile?.pages?.length || 0) > 0 ? 'PROCESSED' : 'FALLBACK'} pages`); - // Convert processed pages to PageEditor format - // All processing is now handled by FileProcessingService when files are added - const pages: PDFPage[] = processedFile?.pages && processedFile.pages.length > 0 - ? processedFile.pages.map((page, index) => { - const pageId = `${primaryFileId}-page-${index + 1}`; - // Try multiple sources for thumbnails in order of preference: - // 1. Processed data thumbnail - // 2. Cached thumbnail from previous generation - // 3. For page 1: FileRecord's thumbnailUrl (from FileProcessingService) - let thumbnail = page.thumbnail || null; - const cachedThumbnail = getThumbnailFromCache(pageId); - if (!thumbnail && cachedThumbnail) { - thumbnail = cachedThumbnail; - console.log(`📸 PageEditor: Using cached thumbnail for page ${index + 1} (${pageId})`); + // Convert processed pages to PageEditor format or create placeholders from metadata + let pages: PDFPage[] = []; + + if (processedFile?.pages && processedFile.pages.length > 0) { + // Use fully processed pages with thumbnails + pages = processedFile.pages.map((page, index) => { + const pageId = `${primaryFileId}-page-${index + 1}`; + // Try multiple sources for thumbnails in order of preference: + // 1. Processed data thumbnail + // 2. Cached thumbnail from previous generation + // 3. For page 1: FileRecord's thumbnailUrl (from FileProcessingService) + let thumbnail = page.thumbnail || null; + const cachedThumbnail = getThumbnailFromCache(pageId); + if (!thumbnail && cachedThumbnail) { + thumbnail = cachedThumbnail; + console.log(`📸 PageEditor: Using cached thumbnail for page ${index + 1} (${pageId})`); + } + if (!thumbnail && index === 0) { + // For page 1, use the thumbnail from FileProcessingService + thumbnail = primaryFileRecord.thumbnailUrl || null; + if (thumbnail) { + addThumbnailToCache(pageId, thumbnail); + console.log(`📸 PageEditor: Using FileProcessingService thumbnail for page 1 (${pageId})`); } - if (!thumbnail && index === 0) { - // For page 1, use the thumbnail from FileProcessingService - thumbnail = primaryFileRecord.thumbnailUrl || null; - if (thumbnail) { - addThumbnailToCache(pageId, thumbnail); - console.log(`📸 PageEditor: Using FileProcessingService thumbnail for page 1 (${pageId})`); - } + } + + return { + id: pageId, + pageNumber: index + 1, + thumbnail, + rotation: page.rotation || 0, + selected: false, + splitBefore: page.splitBefore || false, + }; + }); + } else if (processedFile?.totalPages && processedFile.totalPages > 0) { + // Create placeholder pages from metadata while thumbnails are being generated + console.log(`🎬 PageEditor: Creating ${processedFile.totalPages} placeholder pages from metadata`); + pages = Array.from({ length: processedFile.totalPages }, (_, index) => { + const pageId = `${primaryFileId}-page-${index + 1}`; + + // Check for existing cached thumbnail + let thumbnail = getThumbnailFromCache(pageId) || null; + + // For page 1, try to use the FileRecord thumbnail + if (!thumbnail && index === 0) { + thumbnail = primaryFileRecord.thumbnailUrl || null; + if (thumbnail) { + addThumbnailToCache(pageId, thumbnail); + console.log(`📸 PageEditor: Using FileProcessingService thumbnail for placeholder page 1 (${pageId})`); } - - - return { - id: pageId, - pageNumber: index + 1, - thumbnail, - rotation: page.rotation || 0, - selected: false, - splitBefore: page.splitBefore || false, - }; - }) - : [{ // Fallback while FileProcessingService is working - id: `${primaryFileId}-page-1`, - pageNumber: 1, - thumbnail: getThumbnailFromCache(`${primaryFileId}-page-1`) || primaryFileRecord.thumbnailUrl || null, + } + + return { + id: pageId, + pageNumber: index + 1, + thumbnail, // Will be null initially, populated by PageThumbnail components rotation: 0, selected: false, splitBefore: false, - }]; + }; + }); + } else { + // Ultimate fallback - single page while we wait for metadata + pages = [{ + id: `${primaryFileId}-page-1`, + pageNumber: 1, + thumbnail: getThumbnailFromCache(`${primaryFileId}-page-1`) || primaryFileRecord.thumbnailUrl || null, + rotation: 0, + selected: false, + splitBefore: false, + }]; + } // Create document with determined pages @@ -1123,7 +1154,7 @@ const PageEditor = ({ const displayedPages = displayDocument?.pages || []; return ( - + {showEmpty && ( diff --git a/frontend/src/components/pageEditor/PageThumbnail.tsx b/frontend/src/components/pageEditor/PageThumbnail.tsx index 1e709a863..449824aee 100644 --- a/frontend/src/components/pageEditor/PageThumbnail.tsx +++ b/frontend/src/components/pageEditor/PageThumbnail.tsx @@ -81,7 +81,7 @@ const PageThumbnail = React.memo(({ }: PageThumbnailProps) => { const [thumbnailUrl, setThumbnailUrl] = useState(page.thumbnail); const { state, selectors } = useFileState(); - const { getThumbnailFromCache } = useThumbnailGeneration(); + const { getThumbnailFromCache, requestThumbnail } = useThumbnailGeneration(); // Update thumbnail URL when page prop changes - prevent redundant updates useEffect(() => { @@ -91,27 +91,43 @@ const PageThumbnail = React.memo(({ } }, [page.thumbnail, page.id]); // Remove thumbnailUrl dependency to prevent redundant cycles - // Listen for ready thumbnails from Web Workers (only if no existing thumbnail) + // Request thumbnail generation if not available (optimized for performance) useEffect(() => { - if (thumbnailUrl) { - return; // Skip if we already have a thumbnail + if (thumbnailUrl || !originalFile) { + return; // Skip if we already have a thumbnail or no original file } - // Poll for thumbnail in cache (lightweight polling every 500ms) - const pollInterval = setInterval(() => { - // Check if thumbnail is now available in cache - const cachedThumbnail = getThumbnailFromCache(page.id); - if (cachedThumbnail) { - setThumbnailUrl(cachedThumbnail); - clearInterval(pollInterval); // Stop polling once found - } - }, 500); + // Check cache first without async call + const cachedThumbnail = getThumbnailFromCache(page.id); + if (cachedThumbnail) { + setThumbnailUrl(cachedThumbnail); + return; + } - // Cleanup interval - return () => { - clearInterval(pollInterval); + let cancelled = false; + + const loadThumbnail = async () => { + try { + const thumbnail = await requestThumbnail(page.id, originalFile, page.pageNumber); + + // Only update if component is still mounted and we got a result + if (!cancelled && thumbnail) { + setThumbnailUrl(thumbnail); + } + } catch (error) { + if (!cancelled) { + console.warn(`📸 PageThumbnail: Failed to load thumbnail for page ${page.pageNumber}:`, error); + } + } }; - }, [page.pageNumber, page.id]); // Remove thumbnailUrl dependency to stabilize effect + + loadThumbnail(); + + // Cleanup function to prevent state updates after unmount + return () => { + cancelled = true; + }; + }, [page.id, originalFile, requestThumbnail, getThumbnailFromCache]); // Removed thumbnailUrl to prevent loops // Register this component with pageRefs for animations diff --git a/frontend/src/hooks/useThumbnailGeneration.ts b/frontend/src/hooks/useThumbnailGeneration.ts index 2d1138401..85dd91998 100644 --- a/frontend/src/hooks/useThumbnailGeneration.ts +++ b/frontend/src/hooks/useThumbnailGeneration.ts @@ -1,6 +1,110 @@ -import { useCallback } from 'react'; +import { useCallback, useRef } from 'react'; import { thumbnailGenerationService } from '../services/thumbnailGenerationService'; +// Request queue to handle concurrent thumbnail requests +interface ThumbnailRequest { + pageId: string; + file: File; + pageNumber: number; + resolve: (thumbnail: string | null) => void; + reject: (error: Error) => void; +} + +// Global request queue (shared across all hook instances) +const requestQueue: ThumbnailRequest[] = []; +let isProcessingQueue = false; +let batchTimer: number | null = null; + +// Track active thumbnail requests to prevent duplicates across components +const activeRequests = new Map>(); + +// Batch processing configuration +const BATCH_SIZE = 50; // Process thumbnails in batches of 50 +const BATCH_DELAY = 100; // Wait 100ms to collect requests before processing +const PRIORITY_BATCH_DELAY = 50; // Faster processing for the first batch (visible pages) + +// Process the queue in batches for better performance +async function processRequestQueue() { + if (isProcessingQueue || requestQueue.length === 0) { + return; + } + + isProcessingQueue = true; + + try { + while (requestQueue.length > 0) { + // Sort queue by page number to prioritize visible pages first + requestQueue.sort((a, b) => a.pageNumber - b.pageNumber); + + // Take a batch of requests (same file only for efficiency) + const batchSize = Math.min(BATCH_SIZE, requestQueue.length); + const batch = requestQueue.splice(0, batchSize); + + // Group by file to process efficiently + const fileGroups = new Map(); + + // First, resolve any cached thumbnails immediately + const uncachedRequests: ThumbnailRequest[] = []; + + for (const request of batch) { + const cached = thumbnailGenerationService.getThumbnailFromCache(request.pageId); + if (cached) { + request.resolve(cached); + } else { + uncachedRequests.push(request); + + if (!fileGroups.has(request.file)) { + fileGroups.set(request.file, []); + } + fileGroups.get(request.file)!.push(request); + } + } + + // Process each file group with batch thumbnail generation + for (const [file, requests] of fileGroups) { + if (requests.length === 0) continue; + + try { + const pageNumbers = requests.map(req => req.pageNumber); + const arrayBuffer = await file.arrayBuffer(); + + console.log(`📸 Batch generating ${requests.length} thumbnails for pages: ${pageNumbers.slice(0, 5).join(', ')}${pageNumbers.length > 5 ? '...' : ''}`); + + const results = await thumbnailGenerationService.generateThumbnails( + arrayBuffer, + pageNumbers, + { scale: 1.0, quality: 0.8, batchSize: BATCH_SIZE }, + (progress) => { + // Optional: Could emit progress events here for UI feedback + console.log(`📸 Batch progress: ${progress.completed}/${progress.total} thumbnails generated`); + } + ); + + // Match results back to requests and resolve + for (const request of requests) { + const result = results.find(r => r.pageNumber === request.pageNumber); + + if (result && result.success && result.thumbnail) { + thumbnailGenerationService.addThumbnailToCache(request.pageId, result.thumbnail); + request.resolve(result.thumbnail); + } else { + console.warn(`No result for page ${request.pageNumber}`); + request.resolve(null); + } + } + + } catch (error) { + console.warn(`Batch thumbnail generation failed for ${requests.length} pages:`, error); + // Reject all requests in this batch + requests.forEach(request => request.reject(error as Error)); + } + } + } + } finally { + isProcessingQueue = false; + } +} + /** * Hook for tools that want to use thumbnail generation * Tools can choose whether to include visual features @@ -42,15 +146,83 @@ export function useThumbnailGeneration() { }, []); const destroyThumbnails = useCallback(() => { + // Clear any pending batch timer + if (batchTimer) { + clearTimeout(batchTimer); + batchTimer = null; + } + + // Clear the queue and active requests + requestQueue.length = 0; + activeRequests.clear(); + isProcessingQueue = false; + thumbnailGenerationService.destroy(); }, []); + const requestThumbnail = useCallback(async ( + pageId: string, + file: File, + pageNumber: number + ): Promise => { + // Check cache first for immediate return + const cached = thumbnailGenerationService.getThumbnailFromCache(pageId); + if (cached) { + return cached; + } + + // Check if this request is already being processed globally + const activeRequest = activeRequests.get(pageId); + if (activeRequest) { + return activeRequest; + } + + // Create new request promise and track it globally + const requestPromise = new Promise((resolve, reject) => { + requestQueue.push({ + pageId, + file, + pageNumber, + resolve: (result: string | null) => { + activeRequests.delete(pageId); + resolve(result); + }, + reject: (error: Error) => { + activeRequests.delete(pageId); + reject(error); + } + }); + + // Schedule batch processing with a small delay to collect more requests + if (batchTimer) { + clearTimeout(batchTimer); + } + + // Use shorter delay for the first batch (pages 1-50) to show visible content faster + const isFirstBatch = requestQueue.length <= BATCH_SIZE && requestQueue.every(req => req.pageNumber <= BATCH_SIZE); + const delay = isFirstBatch ? PRIORITY_BATCH_DELAY : BATCH_DELAY; + + batchTimer = window.setTimeout(() => { + processRequestQueue().catch(error => { + console.error('Error processing thumbnail request queue:', error); + }); + batchTimer = null; + }, delay); + }); + + // Track this request to prevent duplicates + activeRequests.set(pageId, requestPromise); + + return requestPromise; + }, []); + return { generateThumbnails, addThumbnailToCache, getThumbnailFromCache, getCacheStats, stopGeneration, - destroyThumbnails + destroyThumbnails, + requestThumbnail }; } \ No newline at end of file diff --git a/frontend/src/services/thumbnailGenerationService.ts b/frontend/src/services/thumbnailGenerationService.ts index aa4df8e3c..0ca278448 100644 --- a/frontend/src/services/thumbnailGenerationService.ts +++ b/frontend/src/services/thumbnailGenerationService.ts @@ -26,7 +26,6 @@ export class ThumbnailGenerationService { private workers: Worker[] = []; private activeJobs = new Map(); private jobCounter = 0; - private isGenerating = false; // Session-based thumbnail cache private thumbnailCache = new Map(); @@ -133,11 +132,11 @@ export class ThumbnailGenerationService { options: ThumbnailGenerationOptions = {}, onProgress?: (progress: { completed: number; total: number; thumbnails: ThumbnailResult[] }) => void ): Promise { - if (this.isGenerating) { - throw new Error('Thumbnail generation already in progress'); - } - - this.isGenerating = true; + // Create unique job ID to track this specific generation request + const jobId = `thumbnails-${++this.jobCounter}`; + + // Instead of blocking globally, we'll track individual generation jobs + // This allows multiple thumbnail generation requests to run concurrently const { scale = 0.2, @@ -207,7 +206,7 @@ export class ThumbnailGenerationService { } catch (error) { return await this.generateThumbnailsMainThread(pdfArrayBuffer, pageNumbers, scale, quality, onProgress); } finally { - this.isGenerating = false; + // Individual job completed, no need to reset global flag } } @@ -401,7 +400,6 @@ export class ThumbnailGenerationService { */ stopGeneration(): void { this.activeJobs.clear(); - this.isGenerating = false; } /** @@ -411,7 +409,6 @@ export class ThumbnailGenerationService { this.workers.forEach(worker => worker.terminate()); this.workers = []; this.activeJobs.clear(); - this.isGenerating = false; this.clearThumbnailCache(); } }