From 35ff0f508cf9cb52048f2ed67bd07f4aec17f2bf Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Tue, 22 Jul 2025 08:15:07 +0300 Subject: Fix GUI background image generation race conditions - Add file.Sync() after image download to ensure data is flushed to disk - Add double-checking in UI updates to prevent wrong card updates - Fix background job completion to reload files when user navigates back - Add file size validation in image display widget - Improve error messages for image loading failures This fixes two issues: 1. 'png: invalid format: not enough pixel data' error when navigating during generation 2. Images not updating when navigating back to a card after background generation completes --- internal/gui/app.go | 77 ++++++++++++++++++++++++++++++++++++++++++------- internal/gui/widgets.go | 19 ++++++++++-- 2 files changed, 83 insertions(+), 13 deletions(-) (limited to 'internal/gui') diff --git a/internal/gui/app.go b/internal/gui/app.go index 6a58286..685c913 100644 --- a/internal/gui/app.go +++ b/internal/gui/app.go @@ -716,11 +716,19 @@ func (a *Application) generateMaterials(word string) { a.mu.Lock() if a.currentWord == word { a.currentAudioFile = audioRes.file + audioFile := audioRes.file + a.mu.Unlock() fyne.Do(func() { - a.audioPlayer.SetAudioFile(audioRes.file) + // Double-check inside the UI update that we're still on the same word + a.mu.Lock() + if a.currentWord == word { + a.audioPlayer.SetAudioFile(audioFile) + } + a.mu.Unlock() }) + } else { + a.mu.Unlock() } - a.mu.Unlock() } // Collect image result @@ -735,11 +743,19 @@ func (a *Application) generateMaterials(word string) { a.mu.Lock() if a.currentWord == word { a.currentImage = imageRes.file + imageFile := imageRes.file + a.mu.Unlock() fyne.Do(func() { - a.imageDisplay.SetImages([]string{imageRes.file}) + // Double-check inside the UI update that we're still on the same word + a.mu.Lock() + if a.currentWord == word { + a.imageDisplay.SetImages([]string{imageFile}) + } + a.mu.Unlock() }) + } else { + a.mu.Unlock() } - a.mu.Unlock() } // Collect phonetic result (UI already updated in the goroutine) @@ -967,19 +983,34 @@ func (a *Application) onRegenerateAudio() { defer a.wg.Done() defer a.decrementProcessing() // Audio processing ends + // Store the word we're generating for + wordForGeneration := a.currentWord + // Get or create context for this card - cardCtx, _ := a.getOrCreateCardContext(a.currentWord) + cardCtx, _ := a.getOrCreateCardContext(wordForGeneration) - audioFile, err := a.generateAudio(cardCtx, a.currentWord) + audioFile, err := a.generateAudio(cardCtx, wordForGeneration) if err != nil { fyne.Do(func() { a.showError(fmt.Errorf("Audio regeneration failed: %w", err)) }) } else { - a.currentAudioFile = audioFile - fyne.Do(func() { - a.audioPlayer.SetAudioFile(audioFile) - }) + // Only update if we're still on the same word + a.mu.Lock() + if a.currentWord == wordForGeneration { + a.currentAudioFile = audioFile + a.mu.Unlock() + fyne.Do(func() { + // Double-check inside the UI update that we're still on the same word + a.mu.Lock() + if a.currentWord == wordForGeneration { + a.audioPlayer.SetAudioFile(audioFile) + } + a.mu.Unlock() + }) + } else { + a.mu.Unlock() + } } fyne.Do(func() { @@ -1897,6 +1928,14 @@ func (a *Application) processWordJob(job *WordJob) { if isCurrentJob { fyne.Do(func() { + // Double-check that we're still on the same job before updating UI + a.mu.Lock() + if a.currentJobID != job.ID { + a.mu.Unlock() + return + } + a.mu.Unlock() + a.audioPlayer.SetAudioFile(audioFile) // Enable audio-related actions a.regenerateAudioBtn.Enable() @@ -1948,6 +1987,14 @@ func (a *Application) processWordJob(job *WordJob) { if isCurrentJob { fyne.Do(func() { + // Double-check that we're still on the same job before updating UI + a.mu.Lock() + if a.currentJobID != job.ID { + a.mu.Unlock() + return + } + a.mu.Unlock() + a.translationEntry.SetText(translation) if imageFile != "" { a.imageDisplay.SetImages([]string{imageFile}) @@ -2022,6 +2069,16 @@ func (a *Application) onJobComplete(job *WordJob) { } else { // This is a background job that completed a.updateStatus(fmt.Sprintf("Background processing completed: %s", job.Word)) + + // Check if user has navigated back to this word + a.mu.Lock() + currentWord := a.currentWord + a.mu.Unlock() + + if currentWord == job.Word { + // User is currently viewing this word, reload the files + a.loadExistingFiles(job.Word) + } } } }) diff --git a/internal/gui/widgets.go b/internal/gui/widgets.go index 6ad428c..df74027 100644 --- a/internal/gui/widgets.go +++ b/internal/gui/widgets.go @@ -73,7 +73,20 @@ func (d *ImageDisplay) SetImage(imagePath string) { } defer file.Close() - img, _, err := image.Decode(file) + // Get file info to ensure it's fully written + stat, err := file.Stat() + if err != nil { + d.imageLabel.SetText(fmt.Sprintf("Error getting file info: %v", err)) + return + } + + // If file size is 0, it might still be writing + if stat.Size() == 0 { + d.imageLabel.SetText("Image file is empty") + return + } + + img, format, err := image.Decode(file) if err != nil { d.imageLabel.SetText(fmt.Sprintf("Error decoding image: %v", err)) return @@ -83,8 +96,8 @@ func (d *ImageDisplay) SetImage(imagePath string) { d.imageCanvas.Image = img d.imageCanvas.Refresh() - // Update label - d.imageLabel.SetText(filepath.Base(imagePath)) + // Update label with format info + d.imageLabel.SetText(fmt.Sprintf("%s (%s)", filepath.Base(imagePath), format)) } // SetImages sets multiple images but only displays the first one -- cgit v1.2.3