From aab6589ca0191b5d3be36fa5cf53ab9a2ddaa631 Mon Sep 17 00:00:00 2001 From: Chris Reddington <791642+chrisreddington@users.noreply.github.com> Date: Fri, 22 Nov 2024 10:09:06 +0000 Subject: [PATCH 1/2] Refactor font handling and embed assets instead of relative path --- stl/generator.go | 2 +- stl/geometry/assets.go | 66 ++++++++++++++ .../geometry/assets}/invertocat.png | Bin .../geometry/assets}/monasans-medium.ttf | Bin .../geometry/assets}/monasans-regular.ttf | Bin stl/geometry/assets_test.go | 86 ++++++++++++++++++ stl/geometry/geometry.go | 4 +- stl/geometry/text.go | 31 +++++-- stl/geometry/text_test.go | 11 +-- 9 files changed, 182 insertions(+), 18 deletions(-) create mode 100644 stl/geometry/assets.go rename {assets => stl/geometry/assets}/invertocat.png (100%) rename {assets => stl/geometry/assets}/monasans-medium.ttf (100%) rename {assets => stl/geometry/assets}/monasans-regular.ttf (100%) create mode 100644 stl/geometry/assets_test.go diff --git a/stl/generator.go b/stl/generator.go index ec5153e..7a43a4f 100644 --- a/stl/generator.go +++ b/stl/generator.go @@ -235,7 +235,7 @@ func generateText(username string, startYear int, endYear int, dims modelDimensi // generateLogo handles the generation of the GitHub logo geometry func generateLogo(dims modelDimensions, ch chan<- geometryResult, wg *sync.WaitGroup) { defer wg.Done() - logoTriangles, err := geometry.GenerateImageGeometry(dims.imagePath, dims.innerWidth, geometry.BaseHeight) + logoTriangles, err := geometry.GenerateImageGeometry(dims.innerWidth, geometry.BaseHeight) if err != nil { // Log warning and continue without logo instead of failing if logErr := logger.GetLogger().Warning("Failed to generate logo geometry: %v. Continuing without logo.", err); logErr != nil { diff --git a/stl/geometry/assets.go b/stl/geometry/assets.go new file mode 100644 index 0000000..685d114 --- /dev/null +++ b/stl/geometry/assets.go @@ -0,0 +1,66 @@ +package geometry + +import ( + "embed" + "os" + + "github.com/github/gh-skyline/errors" +) + +//go:embed assets/* +var embeddedAssets embed.FS + +// writeTempFont writes the embedded font to a temporary file and returns its path. +// The caller is responsible for cleaning up the temporary file. +func writeTempFont(fontName string) (string, func(), error) { + fontBytes, err := embeddedAssets.ReadFile("assets/" + fontName) + if err != nil { + return "", nil, errors.New(errors.IOError, "failed to read embedded font", err) + } + + // Create temp file with .ttf extension to ensure proper font loading + tmpFile, err := os.CreateTemp("", "skyline-font-*.ttf") + if err != nil { + return "", nil, errors.New(errors.IOError, "failed to create temp font file", err) + } + + if _, err := tmpFile.Write(fontBytes); err != nil { + tmpFile.Close() + os.Remove(tmpFile.Name()) + return "", nil, errors.New(errors.IOError, "failed to write font to temp file", err) + } + tmpFile.Close() + + cleanup := func() { + os.Remove(tmpFile.Name()) + } + + return tmpFile.Name(), cleanup, nil +} + +// getEmbeddedImage returns a temporary file path for the embedded image. +// The caller is responsible for cleaning up the temporary file. +func getEmbeddedImage() (string, func(), error) { + imgBytes, err := embeddedAssets.ReadFile("assets/invertocat.png") + if err != nil { + return "", nil, errors.New(errors.IOError, "failed to read embedded image", err) + } + + tmpFile, err := os.CreateTemp("", "skyline-img-*.png") + if err != nil { + return "", nil, errors.New(errors.IOError, "failed to create temp image file", err) + } + + if _, err := tmpFile.Write(imgBytes); err != nil { + tmpFile.Close() + os.Remove(tmpFile.Name()) + return "", nil, errors.New(errors.IOError, "failed to write image to temp file", err) + } + tmpFile.Close() + + cleanup := func() { + os.Remove(tmpFile.Name()) + } + + return tmpFile.Name(), cleanup, nil +} diff --git a/assets/invertocat.png b/stl/geometry/assets/invertocat.png similarity index 100% rename from assets/invertocat.png rename to stl/geometry/assets/invertocat.png diff --git a/assets/monasans-medium.ttf b/stl/geometry/assets/monasans-medium.ttf similarity index 100% rename from assets/monasans-medium.ttf rename to stl/geometry/assets/monasans-medium.ttf diff --git a/assets/monasans-regular.ttf b/stl/geometry/assets/monasans-regular.ttf similarity index 100% rename from assets/monasans-regular.ttf rename to stl/geometry/assets/monasans-regular.ttf diff --git a/stl/geometry/assets_test.go b/stl/geometry/assets_test.go new file mode 100644 index 0000000..4a3c21a --- /dev/null +++ b/stl/geometry/assets_test.go @@ -0,0 +1,86 @@ +package geometry + +import ( + "os" + "path/filepath" + "testing" +) + +// TestWriteTempFont verifies temporary font file creation and cleanup +func TestWriteTempFont(t *testing.T) { + t.Run("verify valid font extraction", func(t *testing.T) { + fontPath, cleanup, err := writeTempFont("monasans-medium.ttf") + if err != nil { + t.Fatalf("writeTempFont failed: %v", err) + } + defer cleanup() + + // Verify file exists and has content + content, err := os.ReadFile(fontPath) + if err != nil { + t.Errorf("Failed to read temp font file: %v", err) + } + if len(content) == 0 { + t.Error("Temp font file is empty") + } + + // Verify file extension + if filepath.Ext(fontPath) != ".ttf" { + t.Errorf("Expected .ttf extension, got %s", filepath.Ext(fontPath)) + } + + // Verify cleanup works + cleanup() + if _, err := os.Stat(fontPath); !os.IsNotExist(err) { + t.Error("Temp font file not cleaned up properly") + } + }) + + t.Run("verify nonexistent font handling", func(t *testing.T) { + _, cleanup, err := writeTempFont("nonexistent.ttf") + if err == nil { + defer cleanup() + t.Error("Expected error for nonexistent font") + } + }) +} + +// TestGetEmbeddedImage verifies temporary image file creation and cleanup +func TestGetEmbeddedImage(t *testing.T) { + t.Run("verify valid image extraction", func(t *testing.T) { + imagePath, cleanup, err := getEmbeddedImage() + if err != nil { + t.Fatalf("getEmbeddedImage failed: %v", err) + } + defer cleanup() + + // Verify file exists and has content + content, err := os.ReadFile(imagePath) + if err != nil { + t.Errorf("Failed to read temp image file: %v", err) + } + if len(content) == 0 { + t.Error("Temp image file is empty") + } + + // Verify file extension + if filepath.Ext(imagePath) != ".png" { + t.Errorf("Expected .png extension, got %s", filepath.Ext(imagePath)) + } + + // Verify cleanup works + cleanup() + if _, err := os.Stat(imagePath); !os.IsNotExist(err) { + t.Error("Temp image file not cleaned up properly") + } + }) + + // Test embedded filesystem access + t.Run("verify embedded filesystem access", func(t *testing.T) { + // Try to read the embedded image directly + _, err := embeddedAssets.ReadFile("assets/invertocat.png") + if err != nil { + t.Errorf("Failed to access embedded image: %v", err) + } + }) +} diff --git a/stl/geometry/geometry.go b/stl/geometry/geometry.go index 073789c..55c7868 100644 --- a/stl/geometry/geometry.go +++ b/stl/geometry/geometry.go @@ -25,8 +25,8 @@ const ( // Font file paths for text rendering. const ( - PrimaryFont = "assets/monasans-medium.ttf" - FallbackFont = "assets/monasans-regular.ttf" + PrimaryFont = "monasans-medium.ttf" + FallbackFont = "monasans-regular.ttf" ) // Additional constants for year range styling diff --git a/stl/geometry/text.go b/stl/geometry/text.go index b60b409..41556d6 100644 --- a/stl/geometry/text.go +++ b/stl/geometry/text.go @@ -111,12 +111,21 @@ func Create3DText(username string, year string, innerWidth, baseHeight float64) // renderText generates 3D geometry for the given text configuration. func renderText(config textRenderConfig) ([]types.Triangle, error) { dc := gg.NewContext(config.contextWidth, config.contextHeight) - if err := dc.LoadFontFace(PrimaryFont, config.fontSize); err != nil { - if err := dc.LoadFontFace(FallbackFont, config.fontSize); err != nil { - return nil, errors.New(errors.IOError, "failed to load fonts", err) + + // Get temporary font file + fontPath, cleanup, err := writeTempFont(PrimaryFont) + if err != nil { + // Try fallback font + fontPath, cleanup, err = writeTempFont(FallbackFont) + if err != nil { + return nil, errors.New(errors.IOError, "failed to load any fonts", err) } } + if err := dc.LoadFontFace(fontPath, config.fontSize); err != nil { + return nil, errors.New(errors.IOError, "failed to load font", err) + } + dc.SetRGB(0, 0, 0) dc.Clear() dc.SetRGB(1, 1, 1) @@ -147,11 +156,19 @@ func renderText(config textRenderConfig) ([]types.Triangle, error) { } } + defer cleanup() + return triangles, nil } -// GenerateImageGeometry creates 3D geometry from a PNG image file. -func GenerateImageGeometry(imagePath string, innerWidth, baseHeight float64) ([]types.Triangle, error) { +// GenerateImageGeometry creates 3D geometry from the embedded logo image. +func GenerateImageGeometry(innerWidth, baseHeight float64) ([]types.Triangle, error) { + // Get temporary image file + imgPath, cleanup, err := getEmbeddedImage() + if err != nil { + return nil, err + } + config := imageRenderConfig{ renderConfig: renderConfig{ startX: innerWidth * imagePosition, @@ -160,10 +177,12 @@ func GenerateImageGeometry(imagePath string, innerWidth, baseHeight float64) ([] voxelScale: defaultImageScale, depth: frontEmbedDepth, }, - imagePath: imagePath, + imagePath: imgPath, height: defaultImageHeight, } + defer cleanup() + return renderImage(config) } diff --git a/stl/geometry/text_test.go b/stl/geometry/text_test.go index c883830..c1500a7 100644 --- a/stl/geometry/text_test.go +++ b/stl/geometry/text_test.go @@ -167,7 +167,7 @@ func TestGenerateImageGeometry(t *testing.T) { }() t.Run("verify valid image geometry generation", func(t *testing.T) { - triangles, err := GenerateImageGeometry(testPNGPath, 100.0, 5.0) + triangles, err := GenerateImageGeometry(100.0, 5.0) if err != nil { t.Fatalf("GenerateImageGeometry failed: %v", err) } @@ -176,15 +176,8 @@ func TestGenerateImageGeometry(t *testing.T) { } }) - t.Run("verify invalid image path", func(t *testing.T) { - _, err := GenerateImageGeometry("nonexistent.png", 100.0, 5.0) - if err == nil { - t.Error("Expected error for invalid image path") - } - }) - t.Run("verify geometry normal vectors", func(t *testing.T) { - triangles, err := GenerateImageGeometry(testPNGPath, 100.0, 5.0) + triangles, err := GenerateImageGeometry(100.0, 5.0) if err != nil { t.Fatalf("GenerateImageGeometry failed: %v", err) } From ac554aebd207a811cbae8995a4f29877bc1c36b4 Mon Sep 17 00:00:00 2001 From: Chris Reddington <791642+chrisreddington@users.noreply.github.com> Date: Fri, 22 Nov 2024 10:24:29 +0000 Subject: [PATCH 2/2] Improve error handling in temporary file operations for fonts and images --- stl/geometry/assets.go | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/stl/geometry/assets.go b/stl/geometry/assets.go index 685d114..6f8023e 100644 --- a/stl/geometry/assets.go +++ b/stl/geometry/assets.go @@ -2,6 +2,7 @@ package geometry import ( "embed" + "fmt" "os" "github.com/github/gh-skyline/errors" @@ -25,14 +26,17 @@ func writeTempFont(fontName string) (string, func(), error) { } if _, err := tmpFile.Write(fontBytes); err != nil { - tmpFile.Close() - os.Remove(tmpFile.Name()) - return "", nil, errors.New(errors.IOError, "failed to write font to temp file", err) + closeErr := tmpFile.Close() + removeErr := os.Remove(tmpFile.Name()) + return "", nil, errors.New(errors.IOError, "failed to write font to temp file", fmt.Errorf("%w; close error: %v; remove error: %v", err, closeErr, removeErr)) + } + if err := tmpFile.Close(); err != nil { + removeErr := os.Remove(tmpFile.Name()) + return "", nil, errors.New(errors.IOError, "failed to close temp font file", fmt.Errorf("%w; remove error: %v", err, removeErr)) } - tmpFile.Close() cleanup := func() { - os.Remove(tmpFile.Name()) + _ = os.Remove(tmpFile.Name()) // Ignore cleanup errors in defer } return tmpFile.Name(), cleanup, nil @@ -52,14 +56,17 @@ func getEmbeddedImage() (string, func(), error) { } if _, err := tmpFile.Write(imgBytes); err != nil { - tmpFile.Close() - os.Remove(tmpFile.Name()) - return "", nil, errors.New(errors.IOError, "failed to write image to temp file", err) + closeErr := tmpFile.Close() + removeErr := os.Remove(tmpFile.Name()) + return "", nil, errors.New(errors.IOError, "failed to write image to temp file", fmt.Errorf("%w; close error: %v; remove error: %v", err, closeErr, removeErr)) + } + if err := tmpFile.Close(); err != nil { + removeErr := os.Remove(tmpFile.Name()) + return "", nil, errors.New(errors.IOError, "failed to close temp image file", fmt.Errorf("%w; remove error: %v", err, removeErr)) } - tmpFile.Close() cleanup := func() { - os.Remove(tmpFile.Name()) + _ = os.Remove(tmpFile.Name()) // Ignore cleanup errors in defer } return tmpFile.Name(), cleanup, nil