Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
562 changes: 562 additions & 0 deletions docs/dp-best-practices.md

Large diffs are not rendered by default.

251 changes: 251 additions & 0 deletions docs/transparency-debugging.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,251 @@
# Asset Transparency Debugging History

**Problem:** Assets showing black backgrounds instead of transparency on rg35xxplus.

**Status:** ✅ **SOLVED** - Works on all platforms (tg5040, rg35xxplus, desktop)

**Note:** This issue is INDEPENDENT of the +1 tier scaling change. The +1 tier change just selects a larger source asset - it doesn't affect transparency handling. We discovered this transparency bug while testing the +1 tier improvement.

---

## Platform Differences

### Working Platform: tg5040
- Resolution: 1280x720 (or 1024x768 Brick variant)
- SDL Version: SDL 1.2
- Pixel Format: TBD (need to verify)

### Failing Platform: rg35xxplus
- Resolution: 640x480 (or 720x720 CubeXX, 720x480 RG34XX)
- SDL Version: SDL 1.2
- Pixel Format: **RGB565 (16-bit, no alpha channel)**
- Key difference: No alpha mask in surface format

---

## Root Cause Analysis

When surfaces don't have an alpha channel (RGB565):
- RGBA(0,0,0,0) transparent black → RGB(0,0,0) solid black
- Alpha channel is discarded during `SDL_ConvertSurface()` to RGB565
- Transparency must be handled differently for non-alpha formats

---

## Attempted Solutions

### Attempt 0: Original +1 tier implementation (NO transparency handling)
**File:** `workspace/all/common/api.c` line 390
**Code:**
```c
int asset_scale = (int)ceilf(gfx_dp_scale) + 1; // +1 tier for quality

// Asset scaling code with NO special transparency handling
SDL_Surface* temp = SDL_CreateRGBSurface(0, w, h, 32, 0, 0, 0, 0);
SDL_Surface* surface = SDL_ConvertSurface(temp, loaded_assets->format, 0);
SDL_FreeSurface(temp);
// No SetAlpha, no FillRect, just basic create/convert/blit
```
**Result:** ✅ Assets visible on rg35xxplus, ❌ but backgrounds were BLACK instead of transparent

**Why it partially worked:**
- Assets loaded and displayed correctly
- But black backgrounds because SDL_CreateRGBSurface with (0,0,0,0) masks defaults to undefined/black pixels
- The issue: we were trying to FIX this black background problem

**Important:** This proves the +1 tier scaling itself works, just need proper transparency

---

### Attempt 1: SDL_SetSurfaceBlendMode() (SDL2 only)
**File:** `workspace/all/common/api.c` lines ~450, 457, 507, 518
**Code:**
```c
SDL_SetSurfaceBlendMode(surface, SDL_BLENDMODE_NONE);
SDL_FillRect(surface, NULL, SDL_MapRGBA(surface->format, 0, 0, 0, 0));
```
**Result:** ❌ Compile error on SDL 1.2 platforms (function doesn't exist)

---

### Attempt 2: Platform-specific #ifdef blocks
**File:** `workspace/all/common/api.c` lines ~450-465, 507-534
**Code:**
```c
#ifdef USE_SDL2
SDL_SetSurfaceBlendMode(surface, SDL_BLENDMODE_NONE);
#else
SDL_SetAlpha(surface, 0, 0); // SDL 1.2
#endif
SDL_FillRect(surface, NULL, SDL_MapRGBA(surface->format, 0, 0, 0, 0));
```
**Result:** ✅ Compiles on both SDL versions, but ❌ still shows black on rg35xxplus

**Why it failed:**
- Filling with RGBA(0,0,0,0) becomes solid black when converted to RGB565
- The FillRect happens BEFORE conversion, so it fills with transparent black
- But after conversion to RGB565, transparent black → solid black

---

### Attempt 3: Use SDLX_SetAlpha() compatibility macro
**File:** `workspace/all/common/api.c` via `workspace/all/common/sdl.h`
**Code:**
```c
SDLX_SetAlpha(surface, 0, 0); // Uses sdl.h compat layer
SDL_FillRect(surface, NULL, SDL_MapRGBA(surface->format, 0, 0, 0, 0));
```
**Result:** ✅ Compiles cleanly, ❌ still shows black on rg35xxplus

**Why it failed:** Same issue as Attempt 2 - FillRect with transparent black becomes solid black in RGB565

---

### Attempt 4: Remove FillRect, rely on SDL blit behavior
**File:** `workspace/all/common/api.c` lines 445-505
**Code:**
```c
// Create RGBA8888 surface
SDL_Surface* temp = SDL_CreateRGBSurface(0, w, h, 32, RGBA_MASK_8888);
// Convert to loaded_assets format (may be RGB565)
SDL_Surface* surface = SDL_ConvertSurface(temp, loaded_assets->format, 0);
SDL_FreeSurface(temp);
// NO FillRect - let SDL handle transparency through blitting
SDL_BlitSurface(source, &src_rect, surface, &dst_rect);
```
**Result:** ❌ Still shows black backgrounds

**Why it failed:**
- Still converting intermediate surfaces to `loaded_assets->format`
- SDL's default alpha-blending causes `src * alpha + dst * (1-alpha)`
- When dst is uninitialized/black, transparent areas blend to black

---

### Attempt 5: Keep gfx.assets in RGBA8888 format ✅ SUCCESS
**File:** `workspace/all/common/api.c` lines 445-520
**Code:**
```c
// Keep everything in RGBA8888 - don't convert to loaded_assets->format
SDL_Surface* gfx.assets = SDL_CreateRGBSurface(0, sheet_w, sheet_h, 32, RGBA_MASK_8888);

// For each asset extraction and scaling:
SDL_Surface* extracted = SDL_CreateRGBSurface(0, src_w, src_h, 32, RGBA_MASK_8888);
SDLX_SetAlpha(loaded_assets, 0, 0); // Disable alpha-blend, copy RGBA directly
SDL_BlitSurface(loaded_assets, &src_rect, extracted, NULL);
SDLX_SetAlpha(loaded_assets, SDL_SRCALPHA, 0); // Re-enable for later

SDL_Surface* scaled = SDL_CreateRGBSurface(0, target_w, target_h, 32, RGBA_MASK_8888);
GFX_scaleBilinear(extracted, scaled); // Direct pixel copy, preserves alpha

SDLX_SetAlpha(scaled, 0, 0); // Disable alpha-blend for copy to sheet
SDL_BlitSurface(scaled, NULL, gfx.assets, &dst_rect);
```
**Result:** ✅ **Works on all platforms!** (tg5040, rg35xxplus, desktop)

**Theory (based on research):**
1. `IMG_Load` returns PNG in native RGBA format with `SDL_SRCALPHA` enabled
2. With `SDL_SRCALPHA` enabled, blitting does alpha-blending: `result = src*alpha + dst*(1-alpha)`
3. When dst is black/uninitialized, transparent pixels (alpha=0) blend to black
4. **Fix:** Call `SDL_SetAlpha(surface, 0, 0)` before blitting to COPY RGBA data directly
5. Keep `gfx.assets` in RGBA8888 - SDL handles RGBA→RGB565 at final screen blit time

**Key insight from SDL 1.2 docs:**
> "When SDL_SRCALPHA is set, alpha-blending is performed. If SDL_SRCALPHA is not set,
> the RGBA data is copied to the destination surface."

**References:**
- [SDL 1.2 PNG Alpha Blit Issue](https://github.com/libsdl-org/SDL-1.2/issues/51)
- [SDL_DisplayFormatAlpha docs](https://www.libsdl.org/release/SDL-1.2.15/docs/html/sdldisplayformatalpha.html)

---

## Key Technical Details

### SDL_ConvertSurface() Behavior
When converting RGBA8888 → RGB565:
- Alpha channel is **discarded** (no space for it in RGB565)
- RGB(0,0,0) in RGB565 = solid black
- Transparency in RGB565 requires **color keying**, not alpha

### Color Keying vs Alpha Blending
**Alpha Blending** (RGBA formats):
- Per-pixel alpha values (0-255)
- Smooth gradients, antialiasing
- Used in: RGBA8888, some RGBA formats

**Color Keying** (RGB formats):
- One specific RGB color = transparent (e.g., magenta RGB(255,0,255))
- Binary transparency (either visible or invisible)
- Used in: RGB565 when alpha not available
- Set with `SDL_SetColorKey(surface, SDL_SRCCOLORKEY, color_key)`

### The rg35xxplus Problem
rg35xxplus uses RGB565, so:
- No alpha channel available
- Our PNG assets have alpha, but it's lost during conversion
- **SDL should handle this automatically** by preserving source alpha during blit
- But pre-filling with "transparent black" becomes solid black in RGB565

---

## Next Steps If Attempt 4 Fails

### Option A: Use color keying for RGB formats
Detect if surface has no alpha mask, and use a specific color as transparent:
```c
if (!surface->format->Amask) {
// RGB format - use color keying
uint32_t magenta = SDL_MapRGB(surface->format, 255, 0, 255);
SDL_SetColorKey(surface, SDL_SRCCOLORKEY, magenta);
SDL_FillRect(surface, NULL, magenta);
}
```

### Option B: Keep surfaces in RGBA until final blit to screen
Don't convert to loaded_assets format until we're done with all transformations:
```c
// Work in RGBA8888 throughout
SDL_Surface* rgba_assets = /* keep in RGBA */;
// Only convert to RGB565 when blitting to screen
```

### Option C: Check what worked before +1 tier change
Revert the +1 tier change temporarily and verify transparency works, then investigate what's different about the asset loading path for the higher tier.

---

## Test Checklist

When testing transparency fixes:

- [ ] Test on rg35xxplus (RGB565, SDL 1.2)
- [ ] Test on tg5040 (working baseline)
- [ ] Test on miyoomini (RGB565, SDL 1.2)
- [ ] Test on desktop (RGBA, SDL2)
- [ ] Verify icons (battery, wifi, brightness, volume) show transparency
- [ ] Verify pill backgrounds don't have black boxes
- [ ] Check button graphics

---

## Build Commands

```bash
# Test specific platforms
make PLATFORM=rg35xxplus build
make PLATFORM=tg5040 build
make PLATFORM=miyoomini build

# Test desktop
make dev
```

---

## References

- RGB565 format: 16-bit color, 5 bits red, 6 bits green, 5 bits blue, **0 bits alpha**
- RGBA8888 format: 32-bit color, 8 bits per channel including alpha
- SDL_ConvertSurface: Converts pixel formats, discards alpha if destination has no Amask
- SDLX_SetAlpha macro: `workspace/all/common/sdl.h:54-67`
22 changes: 21 additions & 1 deletion makefile
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export LOG_FLAGS
MINARCH_CORES_VERSION ?= 20251119
CORES_BASE = https://github.com/nchapman/minarch-cores/releases/download/$(MINARCH_CORES_VERSION)

.PHONY: build test lint format dev dev-run dev-run-4x3 dev-run-16x9 dev-clean all shell name clean setup done cores-download
.PHONY: build test lint format dev dev-run dev-run-4x3 dev-run-16x9 dev-clean all shell name clean setup done cores-download dev-deploy dev-build-deploy

export MAKEFLAGS=--no-print-directory

Expand Down Expand Up @@ -114,6 +114,26 @@ dev-run-4x3:
dev-run-16x9:
@make -f makefile.dev dev-run-16x9

# Deploy to SD card for rapid dev iteration (skips zip/unzip)
# Usage: make dev-deploy - Deploy all platforms
# make dev-deploy PLATFORM=X - Deploy single platform
dev-deploy:
@if [ -n "$(PLATFORM)" ]; then \
./scripts/dev-deploy.sh --platform $(PLATFORM); \
else \
./scripts/dev-deploy.sh; \
fi

# Build and deploy in one shot for dev iteration
# Usage: make dev-build-deploy - Build all platforms and deploy
# make dev-build-deploy PLATFORM=miyoomini - Build and deploy single platform
dev-build-deploy:
@if [ -n "$(PLATFORM)" ]; then \
make common PLATFORM=$(PLATFORM) && ./scripts/dev-deploy.sh --platform $(PLATFORM); \
else \
make all && ./scripts/dev-deploy.sh; \
fi

# Build all components for a specific platform (in Docker)
build:
# ----------------------------------------------------
Expand Down
12 changes: 10 additions & 2 deletions makefile.dev
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ help:
@echo " brew install sdl2 sdl2_image sdl2_ttf"

# macOS native build configuration
PLATFORM = macos
PLATFORM = desktop
SDL_INCLUDES = -I/opt/homebrew/include
SDL_LIBS = -L/opt/homebrew/lib
FAKESD_PATH = $(shell pwd)/workspace/desktop/FAKESD
FAKESD_PATH = $(shell pwd -P)/workspace/desktop/FAKESD

# Aspect ratio configuration (4x3 or 16x9)
# Can be overridden: ASPECT_RATIO=16x9 make dev-run
Expand Down Expand Up @@ -111,6 +111,14 @@ dev-run: dev
@echo "Launching minui..."
@echo "FAKESD path: $(FAKESD_PATH)"
@echo ""
@echo "Keyboard controls:"
@echo " Arrow keys - D-pad navigation"
@echo " Q/W/A/S - Y/X/B/A buttons"
@echo " Enter - Start"
@echo " 4 - Select"
@echo " Space - Menu"
@echo " Backspace - Power (hold to quit)"
@echo ""
@cd workspace/all/minui && ./build/desktop/minui

# Clean build artifacts
Expand Down
7 changes: 6 additions & 1 deletion makefile.qa
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ TEST_INCLUDES = -I tests/support -I tests/support/unity -I workspace/all/common
TEST_UNITY = tests/support/unity/unity.c

# All test executables (built from tests/unit/ and tests/integration/)
TEST_EXECUTABLES = tests/utils_test tests/pad_test tests/collections_test tests/gfx_text_test tests/audio_resampler_test tests/minarch_paths_test tests/minui_utils_test tests/m3u_parser_test tests/minui_file_utils_test tests/map_parser_test tests/collection_parser_test tests/recent_parser_test tests/recent_writer_test tests/directory_utils_test tests/binary_file_utils_test tests/integration_workflows_test
TEST_EXECUTABLES = tests/utils_test tests/pad_test tests/collections_test tests/gfx_text_test tests/audio_resampler_test tests/minarch_paths_test tests/minui_utils_test tests/m3u_parser_test tests/minui_file_utils_test tests/map_parser_test tests/collection_parser_test tests/recent_parser_test tests/recent_writer_test tests/directory_utils_test tests/binary_file_utils_test tests/ui_layout_test tests/integration_workflows_test

# Default targets: use Docker for consistency
test: docker-test
Expand Down Expand Up @@ -284,6 +284,11 @@ tests/binary_file_utils_test: tests/unit/all/common/test_binary_file_utils.c wor
@echo "Building binary file I/O tests..."
@$(CC) -o $@ $^ $(TEST_INCLUDES) $(TEST_CFLAGS) -D_DEFAULT_SOURCE

# Build UI layout / DP system tests (pure math, no mocking needed)
tests/ui_layout_test: tests/unit/all/common/test_ui_layout.c workspace/all/common/log.c $(TEST_UNITY)
@echo "Building UI layout / DP system tests..."
@$(CC) -o $@ $^ $(TEST_INCLUDES) $(TEST_CFLAGS) -lm

# Build integration tests (tests multiple components working together with real file I/O)
tests/integration_workflows_test: tests/integration/test_workflows.c \
tests/integration/integration_support.c \
Expand Down
Loading