diff --git a/CLAUDE.md b/CLAUDE.md index ef4fd61..1994c72 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -60,20 +60,20 @@ gamma-encoded. If you touch either function, keep them in sync. ## Native RAW processor build paths -The canonical C source is `lib/ffi/raw/raw_processor_common.{c,h}`. Three -consumers include/compile it: +The canonical C source is `lib/ffi/raw/raw_processor_common.{c,h}`. All four +consumers compile a tiny wrapper that `#include`s it: -- Production Linux build (CMake) — `linux/raw_processor/raw_processor_wrapper.c` - which `#include`s `../../lib/ffi/raw/raw_processor_common.c`. +- Production Linux build (CMake) — `linux/raw_processor/raw_processor_wrapper.c`. - Test libraries (`scripts/build_test_libs.sh`) — same wrapper. - Flatpak (`dev.myyc.aks.yaml`) — same wrapper. -- macOS (`macos/raw_processor/raw_processor.c`) — **standalone copy**, not - wrapped. If you change the common source, mirror it here too or convert - macOS to use a wrapper the same way. +- macOS (`macos/raw_processor/raw_processor_wrapper.c`) — same wrapper. + `macos/build.sh` and `macos/Makefile` pass `-I../lib/ffi/raw` so the + canonical header is picked up; Xcode's "Build Native Libraries" phase + lists the wrapper + canonical `.c`/`.h` as input dependencies. -All three Linux paths were consolidated onto the wrapper in the -`refactor/cleanup-and-imagestate` branch; prior to that, stale duplicates -at `linux/raw_processor/raw_processor.c` kept drifting. +Consolidation history: Linux paths were unified on the +`refactor/cleanup-and-imagestate` branch; macOS was migrated on the +follow-up `followup/macos-parity-and-tone-curve-test` branch. ## Histogram diff --git a/linux/vulkan_processor/shaders/image_process.comp b/linux/vulkan_processor/shaders/image_process.comp index f63743d..f483c43 100644 --- a/linux/vulkan_processor/shaders/image_process.comp +++ b/linux/vulkan_processor/shaders/image_process.comp @@ -81,21 +81,22 @@ vec3 applyToneCurves(vec3 color) { if (params.toneCurveEnabled == 0.0) { return color; } - - // Convert to 0-255 range - ivec3 indices = ivec3(clamp(color * 255.0, 0.0, 255.0)); - - // Apply RGB master curve first - indices.r = int(getLutValue(rgbLut.data, uint(indices.r))); - indices.g = int(getLutValue(rgbLut.data, uint(indices.g))); - indices.b = int(getLutValue(rgbLut.data, uint(indices.b))); - - // Then apply individual channel curves + + // Round (not truncate) when sampling the 256-entry byte LUTs so + // colour 0.50196 (which is byte 128) hits index 128, not 127. + ivec3 indices = ivec3(clamp(color * 255.0 + 0.5, 0.0, 255.0)); + + // Must match the CPU order (cpu_processor.dart:_applyToneCurve): + // per-channel first, then master RGB. The two orders are not + // commutative for non-identity curves. indices.r = int(getLutValue(redLut.data, uint(indices.r))); indices.g = int(getLutValue(greenLut.data, uint(indices.g))); indices.b = int(getLutValue(blueLut.data, uint(indices.b))); - - // Convert back to 0-1 range + + indices.r = int(getLutValue(rgbLut.data, uint(indices.r))); + indices.g = int(getLutValue(rgbLut.data, uint(indices.g))); + indices.b = int(getLutValue(rgbLut.data, uint(indices.b))); + return vec3(indices) / 255.0; } diff --git a/macos/Makefile b/macos/Makefile index a4ad458..a98b1b7 100644 --- a/macos/Makefile +++ b/macos/Makefile @@ -10,9 +10,10 @@ BUILD_DIR = . # Targets all: libraw_processor.dylib -libraw_processor.dylib: raw_processor/raw_processor.c +libraw_processor.dylib: raw_processor/raw_processor_wrapper.c ../lib/ffi/raw/raw_processor_common.c $(CC) $(CFLAGS) -o $(BUILD_DIR)/libraw_processor.dylib \ - raw_processor/raw_processor.c \ + raw_processor/raw_processor_wrapper.c \ + -I../lib/ffi/raw \ $(LIBRAW_FLAGS) -lm clean: diff --git a/macos/Runner.xcodeproj/project.pbxproj b/macos/Runner.xcodeproj/project.pbxproj index 7b1b8df..22b63bd 100644 --- a/macos/Runner.xcodeproj/project.pbxproj +++ b/macos/Runner.xcodeproj/project.pbxproj @@ -301,8 +301,9 @@ inputFileListPaths = ( ); inputPaths = ( - "$(SRCROOT)/raw_processor/raw_processor.c", - "$(SRCROOT)/raw_processor/raw_processor.h", + "$(SRCROOT)/raw_processor/raw_processor_wrapper.c", + "$(SRCROOT)/../lib/ffi/raw/raw_processor_common.c", + "$(SRCROOT)/../lib/ffi/raw/raw_processor_common.h", ); name = "Build Native Libraries"; outputFileListPaths = ( diff --git a/macos/build.sh b/macos/build.sh index 9702cd1..24ae2d8 100755 --- a/macos/build.sh +++ b/macos/build.sh @@ -84,9 +84,13 @@ if [ -n "$LIBRAW_STATIC" ]; then # Set minimum macOS version to match system libraries # Use 12.0 as a reasonable minimum that supports both Intel and Apple Silicon + # Source is the wrapper which includes the canonical C file at + # ../lib/ffi/raw/raw_processor_common.c — shared across Linux / macOS / + # Flatpak so the FFI surface never drifts across platforms. clang -shared -fPIC -o libraw_processor.dylib \ -mmacosx-version-min=12.0 \ - raw_processor/raw_processor.c \ + raw_processor/raw_processor_wrapper.c \ + -I"../lib/ffi/raw" \ -I"$LIBRAW_INCLUDE" \ "$LIBRAW_STATIC" \ $JPEG_LIB \ @@ -99,7 +103,8 @@ else echo -e "${YELLOW}Using dynamic linking for libraw (may have dependency issues)${NC}" clang -shared -fPIC -o libraw_processor.dylib \ -mmacosx-version-min=12.0 \ - raw_processor/raw_processor.c \ + raw_processor/raw_processor_wrapper.c \ + -I"../lib/ffi/raw" \ -I"$LIBRAW_INCLUDE" -L"$LIBRAW_LIB" -lraw -lm \ -Wl,-rpath,@loader_path fi diff --git a/macos/raw_processor/raw_processor.c b/macos/raw_processor/raw_processor.c deleted file mode 100644 index 3a69ddb..0000000 --- a/macos/raw_processor/raw_processor.c +++ /dev/null @@ -1,222 +0,0 @@ -#include "raw_processor.h" -#include -#include -#include -#include -#include -#include - -static char last_error[256] = {0}; - -void* raw_processor_init() { - libraw_data_t* processor = libraw_init(0); - if (!processor) { - snprintf(last_error, sizeof(last_error), "Failed to initialize LibRaw"); - return NULL; - } - - // Set default processing parameters - processor->params.output_bps = 8; // 8 bits per channel - processor->params.output_color = 1; // sRGB - processor->params.use_camera_wb = 1; // Use camera white balance - processor->params.use_auto_wb = 0; - processor->params.no_auto_bright = 1; // Disable auto-brightening to preserve RAW data - processor->params.output_tiff = 0; - - return processor; -} - -int raw_processor_open(void* processor, const char* filename) { - if (!processor || !filename) { - snprintf(last_error, sizeof(last_error), "Invalid processor or filename"); - return -1; - } - - // Check if file exists and is readable (helps with sandbox issues) - FILE* test = fopen(filename, "rb"); - if (!test) { - snprintf(last_error, sizeof(last_error), "Cannot open file: %s (errno: %d - %s)", - filename, errno, strerror(errno)); - return -1; - } - fclose(test); - - libraw_data_t* lr = (libraw_data_t*)processor; - int ret = libraw_open_file(lr, filename); - - if (ret != LIBRAW_SUCCESS) { - snprintf(last_error, sizeof(last_error), "Failed to open file: %s", libraw_strerror(ret)); - return ret; - } - - ret = libraw_unpack(lr); - if (ret != LIBRAW_SUCCESS) { - snprintf(last_error, sizeof(last_error), "Failed to unpack RAW: %s", libraw_strerror(ret)); - return ret; - } - - return LIBRAW_SUCCESS; -} - -int raw_processor_process(void* processor) { - if (!processor) { - snprintf(last_error, sizeof(last_error), "Invalid processor"); - return -1; - } - - libraw_data_t* lr = (libraw_data_t*)processor; - int ret = libraw_dcraw_process(lr); - - if (ret != LIBRAW_SUCCESS) { - snprintf(last_error, sizeof(last_error), "Failed to process RAW: %s", libraw_strerror(ret)); - return ret; - } - - return LIBRAW_SUCCESS; -} - -RawImageData* raw_processor_get_rgb(void* processor) { - if (!processor) { - snprintf(last_error, sizeof(last_error), "Invalid processor"); - return NULL; - } - - libraw_data_t* lr = (libraw_data_t*)processor; - int error_code = 0; - - libraw_processed_image_t* processed = libraw_dcraw_make_mem_image(lr, &error_code); - if (!processed || error_code != LIBRAW_SUCCESS) { - snprintf(last_error, sizeof(last_error), "Failed to create RGB image: %s", - error_code ? libraw_strerror(error_code) : "Unknown error"); - return NULL; - } - - RawImageData* image = (RawImageData*)malloc(sizeof(RawImageData)); - if (!image) { - libraw_dcraw_clear_mem(processed); - snprintf(last_error, sizeof(last_error), "Memory allocation failed"); - return NULL; - } - - // Calculate actual image data size (without header) - int data_size = processed->data_size; - - // Allocate and copy RGB data - image->data = (uint8_t*)malloc(data_size); - if (!image->data) { - free(image); - libraw_dcraw_clear_mem(processed); - snprintf(last_error, sizeof(last_error), "Memory allocation failed for image data"); - return NULL; - } - - memcpy(image->data, processed->data, data_size); - image->size = data_size; - - // Fill image info - image->info.width = processed->width; - image->info.height = processed->height; - image->info.bits = processed->bits; - image->info.colors = processed->colors; - - libraw_dcraw_clear_mem(processed); - return image; -} - -void raw_processor_free_image(RawImageData* image) { - if (image) { - if (image->data) { - free(image->data); - } - free(image); - } -} - -void raw_processor_cleanup(void* processor) { - if (processor) { - libraw_close((libraw_data_t*)processor); - } -} - -const char* raw_processor_get_error() { - return last_error; -} - -// Extract EXIF metadata from the opened RAW file -ExifData* raw_processor_get_exif(void* processor) { - if (!processor) { - snprintf(last_error, sizeof(last_error), "Invalid processor"); - return NULL; - } - - libraw_data_t* lr = (libraw_data_t*)processor; - - // Allocate EXIF structure - ExifData* exif = (ExifData*)calloc(1, sizeof(ExifData)); - if (!exif) { - snprintf(last_error, sizeof(last_error), "Memory allocation failed for EXIF"); - return NULL; - } - - // Extract camera info - if (lr->idata.make[0] != '\0') { - exif->make = strdup(lr->idata.make); - } - if (lr->idata.model[0] != '\0') { - exif->model = strdup(lr->idata.model); - } - if (lr->idata.software[0] != '\0') { - exif->software = strdup(lr->idata.software); - } - - // Extract lens info - libraw_lensinfo_t* lensinfo = &lr->lens; - if (lensinfo->LensMake[0] != '\0') { - exif->lens_make = strdup(lensinfo->LensMake); - } - if (lensinfo->Lens[0] != '\0') { - exif->lens_model = strdup(lensinfo->Lens); - } - - // Extract shooting info - exif->iso_speed = lr->other.iso_speed; - exif->aperture = lr->other.aperture; - exif->shutter_speed = lr->other.shutter; - exif->focal_length = lr->other.focal_len; - - // Extract 35mm equivalent focal length if available - if (lr->lens.FocalLengthIn35mmFormat > 0) { - exif->focal_length_35mm = lr->lens.FocalLengthIn35mmFormat; - } - - // Extract timestamp (convert to string) - if (lr->other.timestamp > 0) { - char time_str[20]; - struct tm* tm_info = localtime(&lr->other.timestamp); - strftime(time_str, sizeof(time_str), "%Y:%m:%d %H:%M:%S", tm_info); - exif->datetime = strdup(time_str); - } else { - exif->datetime = strdup(""); - } - - // Extract exposure info - using available fields - exif->exposure_program = 0; // Not available in lr->other - exif->exposure_mode = 0; // Not available in lr->other - exif->metering_mode = 0; // Not available in lr->other - exif->exposure_compensation = 0.0; // Not available in lr->other - exif->flash_mode = 0; // Not available in lr->other - exif->white_balance = lr->other.shot_order; // Using shot_order instead of shot_select - - return exif; -} - -void raw_processor_free_exif(ExifData* exif) { - if (exif) { - if (exif->make) free(exif->make); - if (exif->model) free(exif->model); - if (exif->lens_make) free(exif->lens_make); - if (exif->lens_model) free(exif->lens_model); - if (exif->software) free(exif->software); - free(exif); - } -} \ No newline at end of file diff --git a/macos/raw_processor/raw_processor.h b/macos/raw_processor/raw_processor.h deleted file mode 100644 index c033323..0000000 --- a/macos/raw_processor/raw_processor.h +++ /dev/null @@ -1,75 +0,0 @@ -#ifndef RAW_PROCESSOR_H -#define RAW_PROCESSOR_H - -#include - -#ifdef __cplusplus -extern "C" { -#endif - -typedef struct { - int width; - int height; - int bits; - int colors; -} RawImageInfo; - -typedef struct { - uint8_t* data; - int size; - RawImageInfo info; -} RawImageData; - -// EXIF metadata structures -typedef struct { - char* make; - char* model; - char* lens_make; - char* lens_model; - char* software; - int iso_speed; - double aperture; - double shutter_speed; - double focal_length; - double focal_length_35mm; - const char* datetime; - int exposure_program; - int exposure_mode; - int metering_mode; - double exposure_compensation; - int flash_mode; - int white_balance; -} ExifData; - -// Initialize LibRaw processor -void* raw_processor_init(); - -// Open and unpack RAW file -int raw_processor_open(void* processor, const char* filename); - -// Process the RAW image -int raw_processor_process(void* processor); - -// Get RGB image data -RawImageData* raw_processor_get_rgb(void* processor); - -// Extract EXIF metadata -ExifData* raw_processor_get_exif(void* processor); - -// Free image data -void raw_processor_free_image(RawImageData* image); - -// Free EXIF data -void raw_processor_free_exif(ExifData* exif); - -// Cleanup processor -void raw_processor_cleanup(void* processor); - -// Get last error message -const char* raw_processor_get_error(); - -#ifdef __cplusplus -} -#endif - -#endif // RAW_PROCESSOR_H \ No newline at end of file diff --git a/macos/raw_processor/raw_processor_wrapper.c b/macos/raw_processor/raw_processor_wrapper.c new file mode 100644 index 0000000..fe10722 --- /dev/null +++ b/macos/raw_processor/raw_processor_wrapper.c @@ -0,0 +1 @@ +#include "../../lib/ffi/raw/raw_processor_common.c" diff --git a/test/linux/processor_comparison_test.dart b/test/linux/processor_comparison_test.dart index 2206c95..8f607fe 100644 --- a/test/linux/processor_comparison_test.dart +++ b/test/linux/processor_comparison_test.dart @@ -238,7 +238,69 @@ void main() { // Compare - should have minimal difference _comparePixels(baselineResult, curveResult, 'Near-diagonal curve', maxDifference: 5); }); - + + test('CPU and GPU should agree on a non-identity tone curve', () async { + if (rawPixels == null) { + print('SKIPPED: Test image not available'); + return; + } + + if (!await VulkanProcessor.isAvailable()) { + print('SKIPPED: Vulkan not available on this system'); + return; + } + + print('\n=== Testing CPU vs GPU with non-identity tone curve ==='); + + // A mild S-curve with distinct per-channel curves so we exercise + // both the master rgbLut path and the individual red/green/blue LUTs. + final sCurve = ToneCurveAdjustment( + rgbCurve: [ + CurvePoint(0, 0), + CurvePoint(64, 48), // crush shadows + CurvePoint(192, 208), // lift highlights + CurvePoint(255, 255), + ], + redCurve: [ + CurvePoint(0, 0), + CurvePoint(128, 140), // warm shift on reds + CurvePoint(255, 255), + ], + blueCurve: [ + CurvePoint(0, 0), + CurvePoint(128, 116), // cool shift on blues + CurvePoint(255, 255), + ], + ); + + // CPU path + final cpuProcessor = CpuProcessor(); + await cpuProcessor.initialize(); + final cpuResult = await cpuProcessor.processPixels( + Uint8List.fromList(rawPixels), + imageWidth, + imageHeight, + [sCurve], + ); + cpuProcessor.dispose(); + + // GPU path + final gpuProcessor = VulkanProcessor(); + await gpuProcessor.initialize(); + final gpuResult = await gpuProcessor.processPixels( + Uint8List.fromList(rawPixels), + imageWidth, + imageHeight, + [sCurve], + ); + gpuProcessor.dispose(); + + // Shader rounds float→byte with +0.5 before sampling the byte LUT, + // so CPU and GPU produce byte-identical output for tone curves. + _comparePixels(cpuResult, gpuResult, 'S-curve + per-channel', + maxDifference: 1); + }); + test('Exposure adjustment should brighten image', () async { if (rawPixels == null) { print('SKIPPED: Test image not available');