[kcjonson]

Summary

Implement multi-channel signed distance field (MSDF) text rendering foundation to fix z-ordering issues where button text renders behind backgrounds.

This PR completes Phases 1-3 of the SDF text rendering implementation. Phase 4 (batched rendering integration with LayerManager) is in progress and will be completed in this branch.

Phase 1: SDF Atlas Generation Tool ✅

• Build Configuration: Fixed vcpkg msdfgen build by switching to Homebrew LLVM (Apple Clang 17 had libc++ compatibility issues)
• Atlas Generator: Created tools/generate_sdf_atlas executable using msdfgen 1.12.1 API
• Generated Assets:
  • fonts/Roboto-SDF.json - Metadata with glyph metrics and UV coordinates (94 glyphs)
  • fonts/Roboto-SDF.png - 512x512 atlas texture (gitignored, regenerate with generator tool)

Phase 2: FontRenderer SDF Support ✅

• Atlas Loading: LoadSDFAtlas() method loads PNG texture and JSON metadata
• Batch Support: GenerateGlyphQuads() creates glyph quads for batched rendering
• Data Structures: Added SDFGlyph and SDFAtlasMetadata types
• Dual Mode: Supports both legacy FreeType and new SDF atlas rendering

Phase 3: SDF Shaders ✅

• Vertex Shader (msdf_text.vert): Per-vertex color and texture coordinate handling
• Fragment Shader (msdf_text.frag): Median-of-three MSDF algorithm with screen-space derivatives for quality scaling

Technical Details

Build System

• Updated vcpkg.json with msdfgen extensions feature
• Added tools/ subdirectory to CMake
• Build requires: CC=/usr/local/opt/llvm/bin/clang CXX=/usr/local/opt/llvm/bin/clang++

Dependencies

• msdfgen 1.12.1 (from vcpkg)
• nlohmann-json (for metadata parsing)
• stb_image (for PNG loading)

Documentation

• /docs/technical/ui-framework/sdf-text-rendering.md - Complete technical design
• /docs/technical/ui-framework/batched-text-rendering.md - Updated with implementation plan

Remaining Work (Phase 4)

Still in progress on this branch:

• Create TextBatchRenderer class
• Integrate with LayerManager z-ordering
• Update Button component to use batched text rendering
• Testing and validation

Test Plan

Build & Run Atlas Generator:

# Build with Homebrew LLVM
CC=/usr/local/opt/llvm/bin/clang CXX=/usr/local/opt/llvm/bin/clang++ cmake --build build -j8

# Generate atlas (creates fonts/Roboto-SDF.png and fonts/Roboto-SDF.json)
./build/tools/generate_sdf_atlas/generate_sdf_atlas

Verify Compilation:

CC=/usr/local/opt/llvm/bin/clang CXX=/usr/local/opt/llvm/bin/clang++ cmake --build build --target ui -j8

All code compiles successfully. Integration testing will be completed in Phase 4.

[Copilot]

Pull Request Overview

This PR implements the foundation for SDF (Signed Distance Field) text rendering to address critical z-ordering issues where button text renders behind backgrounds. The implementation spans three phases: atlas generation tooling, FontRenderer SDF support, and MSDF shaders, plus additional Button component implementation and input abstraction.

Key Changes:

• Added msdfgen-based atlas generation tool for creating SDF font textures
• Extended FontRenderer with SDF atlas loading and glyph quad generation
• Implemented MSDF vertex and fragment shaders for high-quality text rendering
• Created Button UI component with state management and styling system
• Introduced platform-independent input abstraction layer (Key, MouseButton enums)

Reviewed Changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 22 comments.

Show a summary per file

File	Description
vcpkg.json	Added msdfgen dependency with extensions feature; updated baseline
tools/generate_sdf_atlas/main.cpp	Atlas generator using msdfgen to create SDF textures and metadata
tools/generate_sdf_atlas/CMakeLists.txt	Build configuration for atlas generation tool
libs/ui/font/font_renderer.h	Added SDF data structures (SDFGlyph, SDFAtlasMetadata) and batch methods
libs/ui/font/font_renderer.cpp	Implemented LoadSDFAtlas and GenerateGlyphQuads for SDF rendering
libs/renderer/shaders/msdf_text.vert	Vertex shader for SDF text with per-vertex color/UVs
libs/renderer/shaders/msdf_text.frag	Fragment shader with median-of-three MSDF algorithm
libs/ui/components/button/*	Complete Button component with 5-state styling system
libs/engine/input/input_types.h	Platform-independent Key and MouseButton enumerations
libs/engine/input/input_manager.*	Updated to use type-safe input enums instead of raw GLFW constants
fonts/Roboto-SDF.json	Generated SDF atlas metadata with 94 glyphs
docs/technical/ui-framework/sdf-text-rendering.md	Comprehensive SDF implementation guide
docs/technical/ui-framework/batched-text-rendering.md	Command queue architecture documentation
apps/ui-sandbox/scenes/button_scene.cpp	Interactive demo showcasing Button states and keyboard navigation
apps/ui-sandbox/scenes/input_test_scene.cpp	Updated to use type-safe input enums

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

[nitpick] The atlas generation uses a simple grid layout (lines 159-184) which wastes space when glyphs have different sizes. For a 512x512 atlas with 32px glyph slots, this supports at most 256 glyphs (16x16 grid), but the charset has 94 characters. While this works for the initial set, it's inefficient. Consider using a proper bin-packing algorithm (e.g., shelf packing or the packer suggested in the documentation comments) for better space utilization, especially if more characters are added later.

[Copilot]

[nitpick] The button's z-index calculation zIndex + 0.1F (line 41 in button.cpp) to place text above the background is fragile. If the parent sets a z-index of 10.0, the text will be at 10.1, but if another element at 10.05 exists, it will render between them. Additionally, floating-point precision issues could cause problems. Consider using a more robust layering system or integer-based z-indices with guaranteed spacing.

Suggested change

	.zIndex = zIndex + 0.1F, // Slightly above button background
	.zIndex = zIndex + 1, // Above button background

[Copilot]

The button scene manually calls Renderer::Primitives::EndFrame() before and after rendering (lines 297 and 308). The comment at line 294 correctly identifies this is a workaround for the z-ordering bug that this PR is meant to fix. However, since Phase 4 (batched rendering integration) is incomplete, this workaround is necessary but creates the exact problem described in the batched-text-rendering.md documentation. This temporary solution should be clearly marked as TODO and removed when Phase 4 is complete.

[Copilot]

[nitpick] The generated JSON metadata file uses different numeric precision inconsistently. Some values like "distanceRange": 4 use integers, while glyph metrics use high-precision floats (e.g., "ascender": 0.927734). The atlas coordinates use normalized floats with varying precision. While functionally correct, consider standardizing the precision for consistency and file size optimization (e.g., 6 decimal places for all normalized values).

[Copilot]

The STB_IMAGE_IMPLEMENTATION macro should only be defined once across the entire project to avoid linker errors due to duplicate symbol definitions. If stb_image is used elsewhere in the codebase, this will cause "multiple definition" errors during linking. Consider defining this in a dedicated .cpp file (e.g., stb_image.cpp) or ensuring it's only defined in one translation unit.

Suggested change

#define STB_IMAGE_IMPLEMENTATION

[Copilot]

The atlas generator's warning "Atlas too small for all glyphs" (line 180) is logged but generation continues. This could result in missing glyphs with no clear indication in the output files. Consider either failing the generation with a clear error, or tracking which glyphs were skipped and reporting them in the JSON metadata or a separate log file.

[Copilot]

Input validation missing: The --size command-line argument (line 61) directly converts to int with std::stoi without validation. Invalid inputs (negative numbers, non-numeric strings, values too large) will either throw an exception or produce nonsensical atlas sizes. Add validation to ensure the size is positive and reasonable (e.g., between 64 and 4096).

[Copilot]

Missing error handling: The LoadSDFAtlas function uses array-style indexing on JSON objects (e.g., json["atlas"]["distanceRange"]) without checking if these keys exist. If the JSON file is malformed or missing expected fields, this will throw an exception that isn't caught, potentially crashing the application. Consider using .contains() or .value() with defaults, or wrap in a try-catch block with more specific error messages.

[Copilot]

[nitpick] Magic number without explanation: The pixelRange is hardcoded to 4.0 with only a brief comment "Distance field range in pixels". This value critically affects the quality and appearance of the SDF text rendering. Consider adding more detailed documentation explaining why 4.0 was chosen, what happens with different values, and whether this should be configurable.

[Copilot]

Pull Request Overview

Copilot reviewed 42 out of 42 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull Request Overview

Copilot reviewed 42 out of 42 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The #define STB_IMAGE_IMPLEMENTATION directive appears in font_renderer.cpp as well (line 11). This can cause linker errors if both files are included in the same compilation unit. Consider moving STB image implementation to a dedicated source file to avoid potential ODR violations.

[Copilot]

[nitpick] The imageData unique_ptr uses a custom deleter (stbi_image_free) which is correct. However, consider using std::unique_ptr<unsigned char[], void(*)(void*)> for array semantics, or create a type alias for clarity:

using StbImagePtr = std::unique_ptr<unsigned char, decltype(&stbi_image_free)>;
StbImagePtr imageData(imageDataRaw, stbi_image_free);

[Copilot]

Excessive logging in hot path. Lines 88-96, 114-115, 124, 134, 138, and throughout Flush() use LOG_INFO for every call. This will spam logs during normal operation. Consider using LOG_DEBUG or removing these logs after debugging is complete.

[Copilot]

Pull Request Overview

Copilot reviewed 42 out of 42 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The atlasBounds fields are added but there's a missing NULL check in the JSON export. If a glyph doesn't have atlasBounds data (whitespace characters), the JSON export will still try to write these fields. Consider adding the same NULL check pattern used for atlas and plane fields on lines 371-373.

[Copilot]

Using #define STB_IMAGE_IMPLEMENTATION in a header-included .cpp file can cause linker errors if this file is included in multiple translation units. Consider moving this to a dedicated .cpp file or wrapping it with an include guard check.

The standard practice is:

// In a single .cpp file (e.g., stb_image.cpp)
#define STB_IMAGE_IMPLEMENTATION
#include <stb_image.h>

// In all other files, just include without the define
#include <stb_image.h>

[Copilot]

The msdfgen version override specifies "1.12" but the PR description mentions using "1.12.1". Version mismatches between documentation and configuration can lead to confusion. Consider updating the override to match the actual version used, or update the documentation to reflect "1.12".

[Copilot]

Pull Request Overview

Copilot reviewed 42 out of 42 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull Request Overview

Copilot reviewed 43 out of 43 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The comment says "Both U and V flipped" but the code only flips V (Y-axis). The U coordinates use uvMin.x and uvMax.x correctly without flipping. The comment should say "V flipped to correct upside-down text" or "Y texture coordinates inverted for OpenGL coordinate system".

[Copilot]

Missing atlasBounds field in the generated JSON. According to the development log and the loader code in font_renderer.cpp (lines 387-396), the generator should export atlasBounds to prevent horizontal squishing of narrow glyphs. The current output only includes atlas and plane, but the fix described in development-log.md (2025-11-20 entry) requires atlasBounds to be included.

Suggested change

	"atlas": {"x": 0, "y": 0, "width": 0.0625, "height": 0.0625},
	"plane": {"left": 0.0141602, "bottom": 0, "right": 0.639648, "top": 0.710938},
	"advance": 0.652344
	},
	"B": {
	"atlas": {"x": 0.0625, "y": 0, "width": 0.0625, "height": 0.0625},
	"plane": {"left": 0.0825195, "bottom": 0, "right": 0.566406, "top": 0.710938},
	"advance": 0.623047
	},
	"C": {
	"atlas": {"x": 0.125, "y": 0, "width": 0.0625, "height": 0.0625},
	"plane": {"left": 0.0585938, "bottom": -0.00976562, "right": 0.605469, "top": 0.720703},
	"advance": 0.650879
	},
	"D": {
	"atlas": {"x": 0.1875, "y": 0, "width": 0.0625, "height": 0.0625},
	"plane": {"left": 0.0825195, "bottom": 0, "right": 0.597168, "top": 0.710938},
	"advance": 0.65625
	},
	"E": {
	"atlas": {"x": 0.25, "y": 0, "width": 0.0625, "height": 0.0625},
	"plane": {"left": 0.0825195, "bottom": 0, "right": 0.53418, "top": 0.710938},
	"advance": 0.568359
	},
	"F": {
	"atlas": {"x": 0.3125, "y": 0, "width": 0.0625, "height": 0.0625},
	"plane": {"left": 0.0825195, "bottom": 0, "right": 0.522949, "top": 0.710938},
	"advance": 0.552734
	},
	"G": {
	"atlas": {"x": 0.375, "y": 0, "width": 0.0625, "height": 0.0625},
	"plane": {"left": 0.0595703, "bottom": -0.00976562, "right": 0.60791, "top": 0.720703},
	"advance": 0.681152
	},
	"H": {
	"atlas": {"x": 0.4375, "y": 0, "width": 0.0625, "height": 0.0625},
	"plane": {"left": 0.0825195, "bottom": 0, "right": 0.628906, "top": 0.710938},
	"advance": 0.713379
	},
	"I": {
	"atlas": {"x": 0.5, "y": 0, "width": 0.0625, "height": 0.0625},
	"atlas": {"x": 0, "y": 0, "width": 0.0625, "height": 0.0625},
	"atlasBounds": {"x": 0, "y": 0, "width": 0.0625, "height": 0.0625},
	"plane": {"left": 0.0141602, "bottom": 0, "right": 0.639648, "top": 0.710938},
	"advance": 0.652344
	},
	"B": {
	"atlas": {"x": 0.0625, "y": 0, "width": 0.0625, "height": 0.0625},
	"atlasBounds": {"x": 0.0625, "y": 0, "width": 0.0625, "height": 0.0625},
	"plane": {"left": 0.0825195, "bottom": 0, "right": 0.566406, "top": 0.710938},
	"advance": 0.623047
	},
	"C": {
	"atlas": {"x": 0.125, "y": 0, "width": 0.0625, "height": 0.0625},
	"atlasBounds": {"x": 0.125, "y": 0, "width": 0.0625, "height": 0.0625},
	"plane": {"left": 0.0585938, "bottom": -0.00976562, "right": 0.605469, "top": 0.720703},
	"advance": 0.650879
	},
	"D": {
	"atlas": {"x": 0.1875, "y": 0, "width": 0.0625, "height": 0.0625},
	"atlasBounds": {"x": 0.1875, "y": 0, "width": 0.0625, "height": 0.0625},
	"plane": {"left": 0.0825195, "bottom": 0, "right": 0.597168, "top": 0.710938},
	"advance": 0.65625
	},
	"E": {
	"atlas": {"x": 0.25, "y": 0, "width": 0.0625, "height": 0.0625},
	"atlasBounds": {"x": 0.25, "y": 0, "width": 0.0625, "height": 0.0625},
	"plane": {"left": 0.0825195, "bottom": 0, "right": 0.53418, "top": 0.710938},
	"advance": 0.568359
	},
	"F": {
	"atlas": {"x": 0.3125, "y": 0, "width": 0.0625, "height": 0.0625},
	"atlasBounds": {"x": 0.3125, "y": 0, "width": 0.0625, "height": 0.0625},
	"plane": {"left": 0.0825195, "bottom": 0, "right": 0.522949, "top": 0.710938},
	"advance": 0.552734
	},
	"G": {
	"atlas": {"x": 0.375, "y": 0, "width": 0.0625, "height": 0.0625},
	"atlasBounds": {"x": 0.375, "y": 0, "width": 0.0625, "height": 0.0625},
	"plane": {"left": 0.0595703, "bottom": -0.00976562, "right": 0.60791, "top": 0.720703},
	"advance": 0.681152
	},
	"H": {
	"atlas": {"x": 0.4375, "y": 0, "width": 0.0625, "height": 0.0625},
	"atlasBounds": {"x": 0.4375, "y": 0, "width": 0.0625, "height": 0.0625},
	"plane": {"left": 0.0825195, "bottom": 0, "right": 0.628906, "top": 0.710938},
	"advance": 0.713379
	},
	"I": {
	"atlas": {"x": 0.5, "y": 0, "width": 0.0625, "height": 0.0625},
	"atlasBounds": {"x": 0.5, "y": 0, "width": 0.0625, "height": 0.0625},

[Copilot]

[nitpick] The fallback condition only checks emSize == 1.0 but doesn't validate other critical metrics. If ascender or descender are zero or invalid, text positioning will fail. Consider validating all critical metrics or documenting that only emSize needs fallback handling.